Add support for descriptions in stylelint command comments

[glen-84]

What is the problem you're trying to solve?

Describing/justifying disable directives, so that they aren't used unless necessary, and other developers know why they were used.

What solution would you like to see?

See the equivalent in ESLint:

• New: Description in directive comments eslint/rfcs#33
• Allow explanation of eslint-disable eslint/eslint#11806
• Breaking: description in directive comments (refs eslint/rfcs#33) eslint/eslint#12699

Example:

a {
    // stylelint-disable-next-line declaration-no-important -- A really good reason here.
    color: red !important;
}

[jeddy3]

@glen-84 Thanks for the request and for using the template.

Mimicking the ESLint implementation sounds good to me, i.e. the reason must come after --.

The feature offers an optional, convenient and consistent way for authors to describe the reason for using a disable command. It is a better solution than the old stylelint-disable-reason rule that was deprecated in 8.0.0, as it sits outside of the sphere of rules.

I've labelled the issue as ready to implement. Please consider contributing if you have time.

[glen-84]

Is it sufficient to add the 3rd line here:

const rules = fullText
    .slice(command.length)
    .split('--')[0].trim() // Allow for description (f.e. /* stylelint-disable a, b -- Description */).
    .split(',')
    .filter(Boolean)
    .map((r) => r.trim());

in getCommandRules?

Assumptions:

• Commands will not include two successive hyphens.
• Rule names will not include two successive hyphens.

If so, how many/what tests should be written?

[jeddy3]

Is it sufficient to add the 3rd line here

I suspect it will be.

Assumptions:

Those assumptions are correct. You should add an explicit warning about double hyphens to the rule docs as part of the pull request.

If so, how many/what tests should be written?

Write as many tests as you need to feel confident that the feature works as expected.

You want to add:

• unit test assigning disable range here
• a couple of integration tests here

For example, you'll want to test that */ stylelint-disable foo-bar -- baz-maz */ assigns a disable range for foo-bar, but not baz-maz.

[ota-meshi]

Note that eslint does not interpret it as a description if there is no whitespace before or after the hyphen.

[glen-84]

Is it necessary to have that limitation?

I could change it to .split(' -- ')[0].trim() (note the space on either side), which would also mean that rules could then (in theory) include double hyphens.

Edit: Actually this might be the better option.

[ota-meshi]

I have checked the eslint source code.
If you want the exact same specifications as eslint, use the following regular expression.

.split(/\s-{2,}\s/u)[0].trim()

https://github.com/eslint/eslint/blob/51e42eca3e87d8259815d736ffe81e604f184057/lib/linter/linter.js#L276

If you use this, the following comments are probably also valid:

/* stylelint-disable rule-name
----------
description */

[nex3]

It looks like this isn't working on stylelint.io/demo. Is that just because the demo site uses an out-of-date version of stylelint?

[glen-84]

@nex3 There hasn't been a release since the feature was merged.

[nex3]

Ah, gotcha! 👍