Add allow-single-line-rule-sets option to one-declaration-per-line rule

[BPScott]

Scss-lint's Single Line Per Property (which is functionally identical to our one-declaration-per-line) rule has an option to allow single line rulesets such as:

p { margin: 0; padding: 0; }

I think it would be a useful addition to sass-lint to add this option.

As an extension to this existing functionality I think it could be useful to add an additional configuration option max-properties-for-single-line-ruleset to allow users to limit the amount of properties within a single-line ruleset, defaulting to Infinity. For instance if max-properties-for-single-line-ruleset was set to 1 then p { margin: 0; padding: 0; } would be invalid, but p { margin: 0; } would be acceptable. This would allow people to limit these single-line rulesets to only be used in cases where the number of properties won't affect readability.

This ticket was split from #283

[BPScott]

Related: ticket #288 contains talk of potentially renaming one-declaration-per-line to single-line-per-property to mimic scss-lint's naming convention.

[Snugug]

The way the SCSS Lint rule works, it by default makes an exemption for single line rulesets, and has an option to also warn on them. I personally take the stance that single line rulesets are a code smell, doubly so when you specifically are disallowing multiple declarations per line. I would prefer this usecase be handled by a user wrapping conditional ignores around these lines once #70 lands.

[joshuacc]

@Snugug I think using conditional ignores is fine if something is an exception to the "house style." However, when the house style explicitly allows grouping similar single line rulesets, a configuration option makes more sense.

I'm perfectly happy with forbidding them by default, but I do think it needs to be configurable to adapt to existing codebases using approaches like Idiomatic CSS.

[DanPurdy]

I'm torn! I personally agree with @Snugug that they are a code smell, we have steps at compilation to minify code, original source should be as readable as possible BUT I don't think we as a linter should necessarily be dictating a style (to a point) we should be providing the tools for people to define their own by setting boundaries around best practices.

I think we stick with forbidding single line by default but provide the allow-single-line-rulesets option. The max-properties-for-single-line-ruleset I feel is another rule entirely due to if it was baked into this rule it would dependant on another option being enabled which may be getting too deep and too far away from our small self contained approach.

TLDR:

• Yes to forbid by default
• Yes to allow-~ option
• Not for me for the max-properties-~ option.

[BPScott]

Happy to have this forbidden by default if it does make its way in, or if not, to use the conditional ignore comments. The single-line rulesets are very much an exception to our house rules in my codebases so they're an ideal use-case for the conditional ignores for my code styling.

The max properties thing was me doing a stream-of-consciousness around potential ways to codify my process of when i deem single-line rulesets to be acceptable (i.e. only when they've got one declaration inside them). Having to explicitly opt blocks into allowing single-line rulesets would also force people to be sure the single-line thing is definitely what they want so that would solve my issue equally well.

[DanPurdy]

@Snugug and @bgriffith I'm going to take your lead on this, I can't decide either way. #70 would be ideal and also if you are enabling a rule to disallow single line declarations then it does seem odd to have a way to disable this in certain cases but on the other hand are we being too dictatorial if we don't..

[Snugug]

My vote is to get #70 in. I'm okay being a bit opinionated in our tool if it means our users are more explicit about exceptions in their style guides.

On Oct 10, 2015, at 7:41 PM, Dan Purdy [email protected] wrote:

@Snugug and @bgriffith I'm going to take your lead on this, I can't decide either way. #70 would be ideal and also if you are enabling a rule to disallow single line declarations then it does seem odd to have a way to disable this in certain cases but on the other hand are we being too dictatorial if we don't..

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