Make *-empty-line-before keyword options consistent

[jeddy3]

#1653 (comment) - brought some clarity to how the *-empty-line-before secondary options should work.

A couple of the existing options are inconsistent.

• comment-empty-line-before's "between-comments" → "after-comment"
• at-rule-empty-line-before's "blockless-group" → "blockless-after-blockless"

This is consistent with the existing keywords of "after-comment", "after-single-line-comment" and "first-nested". And the new keywords of "after-custom-property" and "after-declaration".

We can decide whether to leave them be, or change them in the next major release nearer the time. I wanted to write this issue up to highlight these inconsistencies so that any new options we added going forward are consistent.

We've a proposal for a new option in #1366. Perhaps we should change the proposal to be more consistent with these conventions:

• at-rule-empty-line-before's except: "matching-name-blockless-group" → except: "blockless-after-same-name-blockless". The first "blockless" describes the thing itself and "after-same-name-blockless" describes its relationship to its previous sibling.

Sound about right?

Edit: "all-nested" -> "inside-block", "stylelint-commands" -> "stylelint-command"

[jeddy3]

#2188 made we realise that our "first-nested" options should be called "after-opening-brace" for consistency with all the other "after-*" options. I think it's also clearer.

As an aside, I think our *-before rules with their "after-*" options seem to be standing up well as more use cases are discovered and accounted for.

[ntwb]

SGTM...

We could duplicate the option names and save off having to deprecate them immediately, they could live side by side for a while...

[m-allanson]

Based on the above discussion here's a list of rule options that will be affected by this issue. If there's no objections I can (probably) start implementing these changes later in the week.

Rule name	Current option name	Updated option name
at-rule-empty-line-before	"all-nested"	"inside-block"
at-rule-empty-line-before	"blockless-group"	"blockless-after-blockless"
-	-	-
comment-empty-line-before	"between-comments"	"after-comment"
comment-empty-line-before	"stylelint-commands"	"stylelint-command"
-	-	-
at-rule-empty-line-before	"first-nested"	"after-opening-brace"
comment-empty-line-before	"first-nested"	"after-opening-brace"
custom-property-empty-line-before	"first-nested"	"after-opening-brace"
declaration-empty-line-before	"first-nested"	"after-opening-brace"
rule-nested-empty-line-before	"first-nested"	"after-opening-brace"

[jeddy3]

I just looked through the ignore and except options for all the rules and your write-up LGTM. Can you add these options in master, deprecate them in deprecations and remove them in v8?

[m-allanson]

I've made a start on this, but have skipped adding the options to master and jumped straight to adding the new options & deprecating the old options in the deprecations branch.

[hudochenkov]

Are you sure about after-opening-brace? SugarSS doesn't have opening brace, for example. In my opinion, first-nested more understandable.

after-opening-brace is about syntax, the way something written. first-nested is about meaning, what it actually is.

[davidtheclark]

Good point @hudochenkov!

[jeddy3]

Are you sure about after-opening-brace?

Nope :) I had the same thoughts as you when I looked through #2262.

I've been thinking about it for a couple of days.

Every *-empty-line-before rule used to target a construct (e.g. declaration, rule, at-rule), but then we introduced block-closing-brace-empty-line-before - an *-empty-line-before rule that targets a bit of punctuation within a construct (i.e the closing brace of an at-rule, rule or custom property set).

Each of the *-empty-line-before rules that target a construct had options that either target the construct itself ("first-nested") or the preceding whole construct ("after-declaration, "after-at-rule" etc). By converting "first-nested" to "after-opening-brace" we broke that system as "after-opening-brace" references a piece of punctuation. So, I'm in agreement that we should revert to "first-nested".

Where does that leave "block-closing-brace-empty-line-before", especially in the context of #2090 - introducing an "except": ["after-closing-brace]" (originally "except": ["nesting-at-rule]")?

At first, I was struggling to reconcile this, but now I'm beginning to think that the division is right i.e.

• *-empty-line-before rules that target constructs should only have options that reference the construct itself or the preceding whole constructs. (These rules tend to just use .first and prev()). (These rules are applicable to SugarSS).
• *-empty-line-before rules that target punctuation should only have options that target other punctuation i.e. add "except": ["after-closing-brace]" to "block-closing-brace-empty-line-before". (This rule will require a bit more jiggery-pokery e.g. getting the last node of the construct that the targetted punctuation belongs to, and checking if that node has a block or not). (This rule is not applicable to SugarSS).

Sound good?

I can't think of an alternative, other than introducing a bunch of conflicting *-empty-line-after rules.

P.S. @hudochenkov Thanks for raising your concerns btw :) With your work on postcss-sorting and stylelint-order, you've got a tonne of experience with these rules!

[hudochenkov]

Separation of concerns! I agree with you, @jeddy3.

Thanks for raising your concerns btw :) With your work on postcss-sorting and stylelint-order, you've got a tonne of experience with these rules!

Thank you, @stylelint/core! I've learned a lot from you and still learning. I'm happy to make something in return for your knowledge and fantastic tool :)

[alexander-akait]

@davidtheclark @jeddy3 can be made @hudochenkov as Member? he helps us and more contributors better package 🥇
@stylelint/core

[davidtheclark]

@evilebottnawi Sent an invite!

[hudochenkov]

@stylelint/core thank you for your trust! It means a lot to me! Is there any guide or info I should know in a new position?

[jeddy3]

Is there any guide or info I should know in a new position?

@hudochenkov Not sure much. We've just kind of developed an informal standard way of working. For example:

1. Merging simple documentation fixes after just one review.
2. Waiting on two reviews if the PR affects the core or rules before merging.
3. Updating the CHANGELOG (directly via github) straight after merging a PR, and then posting a comment in the PR with the CHANGELOG item.
4. Trying to be courteous and patient in issues.
5. Having said that, feel free to create a "Saved reply" and use that to close any issue that doesn't use the template:

Thanks for creating this issue but we are closing it as issues need to follow our issue template, so that we can clearly understand your particular circumstances.

Please help us to help you by [recreating the issue](https://github.com/stylelint/stylelint/issues/new) using the template.

6. We're spread out across time zones (US, UK, Russia, Australia etc) and so communication is very asynchronous.
7. After a year and half of (arguably crazy) momentum, we're consciously trying to pace the development better. This has been helped by the fact a lot of the core team are particularly busy at the moment with other commitments.
8. Only use milestones on issues and not PRs - easier to track progress.

I think that's about it.

Welcome aboard and enjoy yourself :)

[hudochenkov]

@jeddy3 thank you!

[hudochenkov]

Back to the topic: @m-allanson will you revert changes for first-nested option? Shall I do this?

[jeddy3]

@hudochenkov If you've time, feel free to jump on it. Remembering that four of the changes ("all-nested" etc) are still valid.

@m-allanson Sorry about the revision and the time you lost making the original changes 😞

[ntwb]

Yay, welcome aboard @hudochenkov, great to have you part of the team 😄

Aside: Apologies for not being around much this year, I quit my job of ~21 years New Years Eve 🎉 and I've been busy with the associated tasks of winding down ones own business, and updating/creating my resume for the next phase of my career 😃

[m-allanson]

@hudochenkov welcome :) and @ntwb congratulations :)

Sorry about the revision and the time you lost making the original changes

No problem, it's good to get these right even if the process takes us in a little circle.

[hudochenkov]

Thank you, everyone! :)

I believe we can close this issue, since it completed.

Done in #2213 and #2278.