sasstools / sass-lint

Pure Node.js Sass linting
MIT License
1.77k stars 532 forks source link

Various discrepancies to scss-lint (indentation and property ordering) #464

Open stowball opened 8 years ago

stowball commented 8 years ago

Indentation is much less forgiving in sass-lint when you align code to make it more readable. Take this example:

.is-expanded {
    opacity: 1;
    z-index: 1;
    height: ($share-services__link-size * $share-services__list-row-count) +
            ($share-services__gutter-size * ($share-services__list-row-count * 2)) +
            ($share-services__toggler-height + $share-services__toggler-margin-bottom) +
            ($share-services__inner-margin * 2) + $share-services__inner-arrow-size;
}

will complain that all of the calculations for height should be at the same indentation as height


Using concentric ordering, sass-lint complains about @font-face declarations, whereas scss-lint doesn't.

@font-face {
    font-family: 'Gotham';
    src: url('font-name.eot');
    src: url('font-name.eot?#iefix') format('embedded-opentype'),
         url('font-name.woff2') format('woff2'),
         url('font-name.woff') format('woff'),
         url('font-name.ttf') format('truetype'),
         url('font-name.svg#fontname') format('svg');
    font-weight: normal;
    font-style: normal;
}

will result in warning Expected font-weight, found src property-sort-order

Dru89 commented 8 years ago

I think scss-lint ignores ordering in things like @mixin declarations as well. I'm not an owner here, but in my opinion sass-lint probably should enforce the ordering for things like @font-face and @mixin (though perhaps this should be a configuration option?).

The other change regarding indentation definitely does seem like something that should be handled, though.

DanPurdy commented 8 years ago

The indentation rule isn't exactly working correctly right now there are a few issues here that describe the issues. We're reluctant to rewrite it at the moment until we move to the latest version of our AST so that we can support tabs and a few other edge cases that are currently broken.. Unfortunately the AST gonzales-pe hasn't seen any updates for a while but fingers crossed soon!

Src isn't specified in the concentric sort order list so the rule falls back to alphabetical for unspecified properties as is described in the docs.. If there's a specific suggestion to change this behaviour then we can look into it but right now it's actually functioning correctly as far as I can see. I don't think we should be ignoring declarations inside mixins or special css cases though