New rule: no-trailing-whitespace #605
Conversation
|
@gyoshev yeah, if you could add the rule to the default config that would be great, I can review the rest of this later on 👍 Thanks! |
|
@DanPurdy I've added the rule to the default config, let me know if other changes are needed |
| @@ -35,6 +35,7 @@ rules: | |||
| no-url-protocols: 1 | |||
| no-vendor-prefixes: 1 | |||
| no-warn: 1 | |||
| no-trailing-whitespace: 1 | |||
There was a problem hiding this comment.
Just a small one but you could you put this in alphabetical order with the rest of the disallow rules so as to follow the pattern. Should be above `no-trailing-zero'.
|
@gyoshev Ok this looks good, I've got one comment above but could you also add a comment above each example in the test files so that we know what part the spaces should be present for just for future reference when someone wants to update the tests as a few IDE's can be set to automatically remove trailing whitespace. For example // Two trailing spaces after .foo
.foo {
margin: 2em;Hopefully that makes sense, if you get those in, update with teh latest from develop and then ping me in a comment I'll merge this in for you. Thanks again! 👍 |
|
Probably worth fiddling with the .editorconfig so people using that don't have it automatically trim the whitespace. |
|
Yeah I thought that but for the sake of two test files which are unlikely to be touched often it seemed a bit much, same issue with the line endings settings too. We're going to have to start adding extra tests for all the CRLF line endings due to Gonzales known issues with those.. Definitely worth looking at tweaking these configs in the future though. The comments in the file for now will just make it obvious to everyone what is being tested within the file. |
For SCSS, the rule only needs to check 'space' nodes for trailing whitespace (\s\n). For SASS, it needs to check both space and delimeter nodes, so it accumulates their content internally.
gyoshev
force-pushed
the
feature/no-trailing-whitespace-rule
branch
from
e07e4b2
to
3523f52
Compare
|
Comments have been added, rule order is sorted, and the branch is rebased onto |
|
@gyoshev awesome! Thanks for this. I'll make an issue to investigate overwriting the editorconfig settings for these files. 👍 |
Will fix #596
Do I need to add new rules to
lib/config/sass-lint.yml?DCO 1.1 Signed-off-by: Alex Gyoshev alex@gyoshev.net