Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty-line-between-blocks should have an option to ignore single line blocks #282

Closed
BPScott opened this issue Oct 9, 2015 · 4 comments · Fixed by #285
Closed

empty-line-between-blocks should have an option to ignore single line blocks #282

BPScott opened this issue Oct 9, 2015 · 4 comments · Fixed by #285
Assignees
Milestone

Comments

@BPScott
Copy link

BPScott commented Oct 9, 2015

Sometimes when writing many short blocks where there is a single property (/ mixin / extend) per block it is useful to group them close to each other as they are often inherently related. For instance a set of width classes can be densely packed instead of having a blank line between each class:

.w1-of-12 { width: (1/12) * 100%; }
.w2-of-12 { width: (2/12) * 100%; }
.w3-of-12 { width: (3/12) * 100%; }
.w4-of-12 { width: (4/12) * 100%; }
.w5-of-12 { width: (5/12) * 100%; }
.w6-of-12 { width: (6/12) * 100%; }
.w7-of-12 { width: (7/12) * 100%; }
.w8-of-12 { width: (8/12) * 100%; }
.w9-of-12 { width: (9/12) * 100%; }
.w10-of-12 { width: (10/12) * 100%; }
.w11-of-12 { width: (11/12) * 100%; }
.w12-of-12 { width: (12/12) * 100%; }

Scss-lint's empty-line-between-blocks rule allows these single-line rule sets to sit next to each other as it has a configuration option allow_single_line_rule_sets that defaults to true. The configuration option may be set to off to disallow this exception.

Sass-lint should mimic this behaviour so that I may write the above without raising warnings.

@DanPurdy
Copy link
Member

DanPurdy commented Oct 9, 2015

I'm good with this, makes a lot of sense.

@bgriffith
Copy link
Member

Yep agree with this.

How about ignore-single-line-rulesets as the option? Since I feel you're instructing the rule to ignore them from having to be potentially followed by a newline.

@BPScott
Copy link
Author

BPScott commented Oct 10, 2015

No strong opinion about if we go with ignore-single-line-rulesets over allow-single-line-rule-sets, (I'm just the guy coming up with requests and doing no work :)) though think that rules with this sort of option should be either always positive or always negative across the whole project for the sake of consistency, i.e. all options that are this sort of boolean toggle should be allow-thingor ignore-thing.

Having a quick flick through the docs, no-qualifying-elements has allow-element-with-attribute, allow-element-with-class and allow-element-with-id but that seems to be the only piece of prior-art.

Personally I think naming these toggle options as positive terms (e.g. allow-thing, enable-thing) is clearer as allow: true is easier to parse than the double negative of ignore: false despite both having the same effect.

@DanPurdy
Copy link
Member

Agreed. We have made many rules fully negative in the sense they are named no-* so any exceptions to our rules should be positive. I think consistently having allow also makes it clearer for people scanning through the config file.

Nice catch, I'll put it to @bgriffith and possibly add some documentation on this going forward.

@bgriffith bgriffith added this to the 1.3.0 milestone Oct 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants