Defining whitespace rules

[davidtheclark]

Expanded from #20 (comment)

It seems that in most whitespace rules in #1 it is suggested that users will be able to enter their own whitespace values. Maybe one space, three spaces, newline, carriage return, etc. I'm worried this will add a lot of overhead and complexity in order to accommodate rare edge cases.

Personally I have not yet seen standardized CSS where the convention was to use anything other than one or no spaces in most of those places (around combinators, colons, bang, commas, parentheses). Only braces have varied, and only by adding the possibility of a newline to that of a single space.

Maybe you don't buy that, and think that we need to allow for lots of variations? Anybody feel this way?

If you agree with me, I can think of a few possible ways simplify things:

• Instead of all of these whitespace -before and -after pairs, we could generally use space rules which have before and after options. So instead of media-query-list-comma-before and media-query-list-comma-after we have media-query-list-comma-space; and its options are either { before: [boolean], after: [boolean] } (where true means always and false means never) or true which must provides a standard default (for commas, that would be { before: false, after: true }). If you don't like having before/after options, we could instead keep separate rules but make them space-specific, e.g. media-query-list-comma-space-before and media-query-list-comma-space-after.
• In the case that a particular place needs more options, we could add another rule instead of opening up the existing one. For example, selector-delimiter-before and selector-delimiter-after (first of all, why not "comma" instead of "delimiter"?) ... so that might reasonably be either a space or a newline (though almost certainly not tab, carriage return, four spaces, etc.). We could make selector-delimiter-space and selector-delimiter-newline with before/after options; or, alternately, selector-delimiter-space-before and selector-delimiter-newline-before and their -after counterparts.

[davidtheclark]

A few notes about prior art:

• Looks like JSCS leans towards splitting up before/after and relevant spacing in separate rules. So we see requireNewlineBeforeBlockStatements and requireSpaceBeforeBlockStatements, disallowSpaceBeforeKeywords and disallowSpaceAfterKeywords. JSCS also, by the way, gets around the boolean options ambiguity (boolean options: true/false vs always/never vs only true #38) by separating allow and disallow rules; so these requires have disallow counterparts. JSCS has not built in accommodation for super weird use-cases, like some guy who wants to require newlines after the opening parenthesis of function parameters.
• SCSS-Lint is very limited in their space enforcement. Seems they just don't bother with rules for enforcing things that are almost universally practiced, such as no space before commas. They have a bunch of SpaceBefore and SpaceAfter rules that have inconsistent options: often just turns on/off to enforce one space, sometimes has an integer for multiple spaces, sometimes has keywords for a bunch of other possibilities.
• ESLint is also kind of inconsistent. They tend towards space-somewhere names that accept always and never keywords to enforce single space or no space, and then arbitrary other keywords for non-standard stuff.

Of the three above, JSCS's consistency and simplicity appeal to me; and I think that the suggestions I've made are closest to it.

[MoOx]

I'm worried this will add a lot of overhead and complexity in order to accommodate rare edge cases.

On space is fine for me for now. It was just an idea.

So instead of media-query-list-comma-before and media-query-list-comma-after we have media-query-list-comma-space; and its options are either { before: [boolean], after: [boolean] }

Good idea.

For example, selector-delimiter-before and selector-delimiter-after (first of all, why not "comma" instead of "delimiter"?)

comma is fine. Maybe I was thinking about including combinators etc.

(though almost certainly not tab, carriage return, four spaces, etc.)

Yep

We could make selector-delimiter-space and selector-delimiter-newline with before/after options; or, alternately, selector-delimiter-space-before and selector-delimiter-newline-before and their -after counterparts.

No sure what is the best. Maybe try with rules without -before/after like first proposition.

Of the three above, JSCS's consistency and simplicity appeal to me; and I think that the suggestions I've made are closest to it.

lgtm too.

I think having one option with an {before/after} object is a good idea.

[jeddy3]

Good thinking! I ended up adding a "nothing" keyword option to declaration-colon-before [https://github.com//pull/43] which felt totally wrong, so I'm eager to standardise our options.

I'm worried this will add a lot of overhead and complexity in order to accommodate rare edge cases.

Agreed. Lets keep it simple.

Maybe you don't buy that, and think that we need to allow for lots of variations? Anybody feel this way?

Nope.

In the case that a particular place needs more options, we could add another rule instead of opening up the existing one.

Agreed.

I think we're totally on the right track, but it doesn't feel like we've adequately addressed the mismatch of boolean options ( #38 ). How about we reserve boolean options for *-no-* rules and use the keywords (e.g. always and never) for the explicit rules? For example:

media-query-list-comma-space-before: "always"|"never",
selector-comma-newline-before: "always"|"never",
color-hex-shorthand: "always"|"never",

color-no-named: bool // only "true" is documented

It doesn't have the cleanliness of JSCS boolean approach, but it feels like an alright compromise to tackle the mismatch of boolean options. How does that sound?

If you don't like having before/after options, we could instead keep separate rules but make them space-specific,

I feel that keeping the rules laser-focused to begin with is the way to go. And so, I'd prefer to keep them separate. Mainly because being laser-focused feels right, but also (to a lesser extent) because:

1. A user might want to enforce whitespace before something, but not after something (and vice-versa). Combining them together into one rule creates an all-or-nothing situation, albeit a edgy-case one.
2. Once we have the laser-focused rules, can't we then just (if the need arises) create wrapper rules around them e.g. declaration-colon-space: { before: [keyword], after: [keyword] } wraps around declaration-colon-space-before: [keyword] and declaration-colon-space-after: [keyword].

Thoughts?

[jeddy3]

Or, another option is following the JSCS allow/disallow to the letter, which makes all booleans true on must be e.g.

media-query-list-comma-space-before: bool // only "true" is documented
media-query-list-comma-no-space-before: bool // only "true" is documented

selector-comma-newline-before: bool // only "true" is documented
selector-comma-no-newline-before: bool // only "true" is documented

color-hex-shorthand: bool // only "true" is documented
color-hex-longhand: bool // only "true" is documented

color-no-named: bool // only "true" is documented

string-quotes: "single"|"double"|"none"

Or is that too odd?

[MoOx]

Agree with your 1. and 2. @jeddy3.
If we choose jscs approch I am afraid we will have "huge-fucking-rule-name-no-yes-not-sure" :D
But I don't know what is the best

[jeddy3]

If we choose jscs approch I am afraid we will have "huge-fucking-rule-name-no-yes-not-sure" :D

Ha, yes, that is a possibility :)

But I don't know what is the best

Me neither. Each approach seems to have its pros and cons.

Thinking about it some more, perhaps the boolean options ambiguity ( #38 ), i.e. inconsistent "must be" and "must not be", isn't a big enough issue that it should force a change to @davidtheclark proposal of using booleans in whitespace rules (e.g. declaration-colon-space-before: [boolean]) because, I think, the boolean options are kind of consistent in that the must and must not are only reversed for *-no-* rules. Which, I guess kind of makes sense as you're saying true to no.

For example:

• color-hex-shorthand: bool
  • true - hex colors must be shorthand
  • false - hex colors must not be shorthand
• color-no-named: bool Disallow named colors
  • true: named colors must not be used for colors i.e true + no = "must not"

Does that make sense or am I just clutching at straws? :)

[davidtheclark]

As noted here #38 (comment) I think part of escaping the boolean ambiguity will be using severity levels to turn on/off. If we do that and use always/never (everyone seemed to like that) we might not really have to use booleans must at all.

declaration-colon-space: 2 would just mean "turn it on with default values". Alternately, you could to could do declaration-colon-space: [2, { before: "always", after: "never" }] if want to fight punctuation norms.

I think I would prefer not splitting up require/disallow and not splitting up before/after in most cases, because looking at JSCS config files makes me want to give up and go to sleep :) --- I think the idea that we'd have a shorter, more clear config.

[davidtheclark]

So if we're in agreement that a 0-1-2 severity level thing is important, I could try to build that in next time I have a few minutes to code. We won't have it affecting any reporter yet; but we'd have it affecting on/off state of rules. Agreed?

[jeddy3]

So if we're in agreement that a 0-1-2 severity level thing is important

Yeap.

We won't have it affecting any reporter yet; but we'd have it affecting on/off state of rules

Yeap, affecting the reporter can come later.

---

I think part of escaping the boolean ambiguity will be using severity levels to turn on/off. If we do that and use always/never (everyone seemed to like that) we might not really have to use booleans must at all.

Ooooh, now I see what you mean. SGTM.

I think I would prefer not splitting up require/disallow and not splitting up before/after in most cases, because looking at JSCS config files makes me want to give up and go to sleep :) --- I think the idea that we'd have a shorter, more clear config.

Ha, fair point :) It was very easy configuring the eslint whitespace rules yesterday as the config was clear and short.

The whole "A user might want to enforce whitespace before something, but not after something (and vice-versa)" felt like an edge-case anyway. If the need ever arises, we could always pull a few of the rules apart and offer them up too. So, going with:

declaration-colon-space: [2, { before: "always", after: "never" }]

Sounds great to me.

The only bit I was unsure about was:

declaration-colon-space: 2 would just mean "turn it on with default values"

Were we going to avoid default rules and values? Or, is it odd to require options for some rules e.g. whitespace ones or things like color-hex-shorthand?

Otherwise, it all makes sense to me. However, just to make doubly sure we're on the same page, the rules would look something like?:

color-hex-shorthand: [2, "always"|"never"],
color-no-named: 2,
media-query-list-comma-space: [2, { before: "always"|"never", after: "always"|"never" }],
url-quotes: [2, "single"|"double"|"none"]

[davidtheclark]

Your example rule-configs sum up nicely. I think we're on the same page.

I think that default values for specific rules that are turned on makes sense, just not having any rules turned on by default. Especially since, as you agree, for many of our cases configuring something other than the default (e.g. no space before comma, one space after) would be unusual.

[jeddy3]

Your example rule-configs sum up nicely. I think we're on the same page.

Good stuff :)

I think that default values for specific rules that are turned on makes sense, just not having any rules turned on by default. Especially since, as you agree, for many of our cases configuring something other than the default (e.g. no space before comma, one space after) would be unusual.

Eek, I meant I'd prefer no default values... :)

I said this over in #1 (comment) about default rules:

The rules within Style Guides tend to be very subjective and distancing the linter from this subjectiveness might help with the clarity and longevity of the project. The linter documentation can then be narrowly focused on describing what the rule does without getting sucked into the murky world of rationalising why (dis)enabling a rule is subjectively better than not.

I think this can be expanded to include the why of default values too.

Take url-quotes for example. scsslint's default is for single quotes as they are "consistent with using other Sass asset helpers." On the otherhand, the google css styleguide advocates no quotes for URLs. What would ours be for url-quotes: 2?

Or take number-leading-zero as another. scsslint's default is for no leading zero as it is "bad and unnecessary". Whereas the SASS Guidelines advocates a leading zero (I assume for readability). Is our number-leading-zero: 2 going to default to always or never?

I think, more often than not, the default won't be as obvious as declaration-colon-space: [2, { before: "always", after: "never" }], but even then I know people who would argue that declaration-colon-space: [2, { before: "always", after: "always" }] should be the default.

I think we're opening ourselves up to a can of worms if we go down the default value route: additional (and subjective) initial discussions, fielding issues about the defaults, additional documentation explaining why the default etc. Being unopinionated (from no default rules down to no default values) frees us from this.

Later down the line we could use the options validator to let the user know when options are required. I think, for now, we can rely on the documentation for each rule. Likewise, offering example configs for a popular styleguide or two would help.

What do you think?

[davidtheclark]

I'm good with no defaults, too, to save the trouble.

[MoOx]

I agree with all things you agree :)

I think I would prefer not splitting up require/disallow and not splitting up before/after in most cases, because looking at JSCS config files makes me want to give up and go to sleep :) --- I think the idea that we'd have a shorter, more clear config.
+1

@jeddy3 example à la ESLint lgtm

I'm good with no defaults, too, to save the trouble.

+1

Great discussion. Thanks guys!

[davidtheclark]

Please review commits 64f72fe and eac5549

I didn't do separate PRs because I needed the first one merged before the second one, before the second is following up on the proof-of-concept. But I'm happy to change stuff as desired.

So here's what I was thinking: We have a lot of whitespace rules that we want to treat in a standard way. There are some subtleties to the whitespace rules that we usually want to repeat, as well (e.g. enforce only one space before instead of at least one space). So I made some utils for the repeating pattern. I figure it might be better to have reusable abstractions than doing a lot of copy-pasting in order to reproduce patterns.

standardWhitespaceOptions and standardWhitespaceMessages just create standardized options and messages objects with an API that standardWhitespaceChecker understands. As you can see in selector-combinator-space and declaration-bang-space, that standardWhitespaceChecker handles the bulk of the validation, reducing boilerplate and possible inconsistencies (e.g. I might have forgotten to check for double spaces in declaration-bang-space).

After creating selector-combinator-space it was a matter of just a few minutes to create declaration-bang-space --- all the time went into writing tests.

[davidtheclark]

I'll wait to check off that those rules are done to see first what you think of this setup.

[MoOx]

I didn't do separate PRs because I needed the first one merged before the second one, before the second is following up on the proof-of-concept

You can do PR cutted from another PR and just tell which one need to be reviewed/merged before the other :)

Very nice approch !

[jeddy3]

Very nice approch !

Agreed. It looks excellent! I'll refactor declaration-colon-space accordingly. I'm learning so much :)

Thanks, as always, for all the great code! :)

[davidtheclark]

Sounds like you're onboard with the approach exemplified in a couple of rules now, so I'll close this.

[davidtheclark]

I have another thought about this:

In the block-brace rules we still have a single-line vs. multi-line distinction which I think we glossed over ... I noticed this when looking back at #1 after @jeddy3's updates.

I'm thinking that maybe the single vs. multi distinction could be made by alternative keywords. We'd add to always and never in certain situations only keywords always-single-line an always-multi-line, never-single-line and never-multi-line. Think that might work?

[jeddy3]

In the block-brace rules we still have a single-line vs. multi-line distinction which I think we glossed over

Ha, yeah. That bit is littered with "(???)"'s :)

Think that might work?

SGTM.

[MoOx]

I'm thinking that maybe the single vs. multi distinction could be made by alternative keywords. We'd add to always and never in certain situations only keywords always-single-line an always-multi-line, never-single-line and never-multi-line. Think that might work?

I think eslint is doing that too (example: comma-dangle: [2, "always-multiline"]). So lgtm !
Note eslint have always & always-multine (no single-line).

[davidtheclark]

Ok sounds like agreement then. I'll close this again.