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
Add configurationComment
configuration property
#6629
Add configurationComment
configuration property
#6629
Conversation
🦋 Changeset detectedLatest commit: e975776 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5d4258b
to
f3cb94c
Compare
disablesPrefix
configuration property
This page will have to be updated. |
f3cb94c
to
f83ff33
Compare
Updated the docs and changed the name of the configuration property to |
5cccf34
to
d2ba0dc
Compare
d2ba0dc
to
448b3e0
Compare
Considering we have also |
@ybiquitous See #6628 (comment) as |
448b3e0
to
41d0722
Compare
We may apply this to more than disables and enables in the future, e.g. configuring rules. So, on further thought, maybe there is a better name than ESLint uses "configuration comments" to describe disabling comments too. Should we adopt that terminology for the option name (and internally so that things are consistent)? It's aligned with our language of configuring rules. The option could be {
"configurationComment": "foo-stylelint"
} Internally, we'd change:
We, rather than the user, should add a hyphen before any of our existing command, e.g.:
How does that sound? |
41d0722
to
c3a055a
Compare
Is there any concern that changing the names of |
98695e5
to
f6d7b44
Compare
@jeddy3's idea sounds good to me. 👍🏼
I believe there is no concern because Just in case, I searched the usage of |
Awesome thanks for looking into that @ybiquitous! Updated those functions and filenames to |
f6d7b44
to
5b50973
Compare
disablesPrefix
configuration propertyconfigurationComment
configuration property
configurationComment
configuration propertyconfigurationComment
configuration property
5b50973
to
eb6f082
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making the changes. It's shaping up nicely.
I've requested a couple of documentation changes and asked one questino.
I pushed 3 commits to standardise the terminology elsewhere in our docs as part of this PR. |
#### `"stylelint-commands"` | ||
|
||
Ignore comments that deliver commands to stylelint, e.g. `/* stylelint-disable color-no-hex */`. | ||
Ignore configuration comments, e.g. `/* stylelint-disable color-no-hex */`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't realised we had an option related to commands. I think we're ok to move forward with "configuration comment", though, as we have a few other rule secondary options which are at odds with our conventions.
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. LGTM 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you.
Closes #6628
This PR changes the list of stylelint commands to omit the prefix, and updates functions related to stylelint commands to accept the prefix as an argument. Additionally it makes stylelintCommandPrefix configurable to the users needs.