Skip to content
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 selector-attribute-name-disallowed-list #4992

Merged
merged 6 commits into from Nov 17, 2020
Merged

Add selector-attribute-name-disallowed-list #4992

merged 6 commits into from Nov 17, 2020

Conversation

rletsin
Copy link
Contributor

@rletsin rletsin commented Oct 18, 2020

Solution for Issue #4915

Is there anything in the PR that needs further explanation?

I think no, it's self-explanatory.

@rletsin
Copy link
Contributor Author

rletsin commented Oct 19, 2020

I'll try to fix linting and test errors.
At the moment I have some trouble reproducing them locally.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rletsin Thanks. It's looks great so far!

In my original proposal, I should have said use both the:

rules as a blueprints; taking the AST traversal from selector-attribute-operator-disallowed-list and the options from media-feature-name-disallowed-list. As this rule targets names, rather than operators, it'll need to accept regexes like the media-feature-name-disallowed-list rule does.

Can you update the options, docs and tests so they are more aligned with media-feature-name-disallowed-list rule, please?

@@ -173,6 +173,7 @@ Grouped first by the following categories and then by the [_thing_](http://apps.
- [`selector-attribute-operator-blacklist`](../../../lib/rules/selector-attribute-operator-blacklist/README.md): Specify a list of disallowed attribute operators. **(deprecated)**
- [`selector-attribute-operator-disallowed-list`](../../../lib/rules/selector-attribute-operator-disallowed-list/README.md): Specify a list of disallowed attribute operators.
- [`selector-attribute-operator-whitelist`](../../../lib/rules/selector-attribute-operator-whitelist/README.md): Specify a list of allowed attribute operators. **(deprecated)**
- [`selector-attribute-name-disallowed-list`](../../../lib/rules/selector-attribute-name-disallowed-list/README.md): Specify a list of disallowed attribute names.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's order this alphabetically, i.e. put it before the operator rules.

@@ -274,6 +274,9 @@ const rules = {
'selector-attribute-operator-whitelist': importLazy(() =>
require('./selector-attribute-operator-whitelist'),
)(),
'selector-attribute-name-disallowed-list': importLazy(() =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's order this alphabetically, i.e. put it before the operator rules.

@jeddy3 jeddy3 changed the title Add 'selector-attribute-name-disallowed-list' rule Add selector-attribute-name-disallowed-list Nov 1, 2020
@rletsin
Copy link
Contributor Author

rletsin commented Nov 9, 2020

@jeddy3 Thanks for your suggestions!
I've added regex as possible input for this rule.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes!

Looks good to me.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice! 👏

@jeddy3 jeddy3 merged commit c36b8d0 into stylelint:master Nov 17, 2020
@jeddy3
Copy link
Member

jeddy3 commented Nov 17, 2020

Changelog:

  • Added: selector-attribute-name-disallowed-list rule (#4992).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants