Newline-related rules giving false positives on Windows for projects with Unix line endings

[joshuacc]

For example, using sass-lint 1.2.0 via gulp-sass on Windows, but using Unix-style LF for EOL:

.test,
.test2 {
    display: block;
}

The documentation seems to indicate that the code above should pass, but it results in this:

src\test.scss: line 1, col 7, Warning - Selectors must be placed on new lines (single-line-per-selector)

1 problem

When I directly modified the rule to replace os.EOL with '\n', the rule worked correctly.

Unfortunately, the user's OS is not a reliable indicator of the project's line endings. I work on a cross-platform team that has standardized on Unix line endings for all code, and I imagine that there are lots of others out there.

[DanPurdy]

https://github.com/sasstools/scssparse/blob/homerolled/lib/re.js#L3

may have to do a match on these instead..

[joshuacc]

FYI, after cloning the project and running the tests on Windows, I get 12 test failures. They all appear newline-related.

[DanPurdy]

Thanks @joshuacc, will definitely look into this a bit further.

[Snugug]

Maybe we need an option to let users set (and enforce) their line ending type?

On Sep 10, 2015, at 7:20 PM, Dan Purdy notifications@github.com wrote:

Thanks @joshuacc, will definitely look into this a bit further.

—
Reply to this email directly or view it on GitHub.

[DanPurdy]

So OS by default but the option to specify? That makes sense to me.

[Snugug]

Actually, this is what I’m thinking:

All of our current newline rules; whatever the first found newline is what it should use for the parsing of that rule. If there is a mix, warn on mixed, but have it be whatever the first found one is.

Add a rule to enforce a specific type of newline, which defaults to OS

On Thursday, September 10, 2015 at 7:37 PM, Dan Purdy wrote:

So OS by default but the option to specify? That makes sense to me.

—
Reply to this email directly or view it on GitHub (#149 (comment)).

[DanPurdy]

Interesting, I still think the option to specify should be there for those that want to standardise everything from the get go but it would definitely make sense for those who haven't tweaked their config to the max, will also avoid any issues across teams.

[joshuacc]

My take on this is that there should be a specific rule for newline characters, but other rules should be character agnostic (even allowing mixed in a single file). That seems more consistent with the small rule philosophy I've seen so far in sass-lint.

[DanPurdy]

Makes sense, looking at the prevalence of checking for os.EOL it could cause a few issues moving forward.

https://github.com/sasstools/sass-lint/search?utf8=%E2%9C%93&q=EOL

[joshuacc]

Would you be open to a PR which fixes this problem in the newline agnostic way I mentioned previously? If so, I can throw one together quickly. 😄

[DanPurdy]

Hey @joshuacc do you want this issue shut down now or are there still some hangovers that need to be resolved?

[joshuacc]

There is still a failing test on Windows with the indentation rule, which seems related to the os.EOL check. However, I haven't seen any actual bugs with that, so maybe track as a separate issue?

[joshuacc]

Created a separate issue #180 for the failing test. Closing this one.