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 splitList: bool to selector-nested-pattern #2430

Closed
aaronjensen opened this issue Mar 17, 2017 · 3 comments · Fixed by #6896
Closed

Add splitList: bool to selector-nested-pattern #2430

aaronjensen opened this issue Mar 17, 2017 · 3 comments · Fixed by #6896
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: new option a new option for an existing rule

Comments

@aaronjensen
Copy link

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

A bug

Which rule, if any, is this issue related to?

selector-nested-pattern

What CSS is needed to reproduce this issue?

.baz {
  .foo,
  .bar {
    color: pink;
  }
}

What stylelint configuration is needed to reproduce this issue?

e.g.

{
  "rules": {
    "selector-nested-pattern": "^.*$"
  }
}

Which version of stylelint are you using?

e.g. 7.1.0

How are you running stylelint: CLI, PostCSS plugin, Node API?

CLI with stylelint --config myconfig *.css

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

Yes, it's related to postcss/scss nesting

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors you are getting)?

The following warnings were flagged:

Expected nested selector ".foo,   .bar" to match specified pattern   selector-nested-pattern
@hudochenkov
Copy link
Member

Thank you for using a template.

It's not a bug. From rule description:

The selector value will be checked in its entirety. If you'd like to allow for combinators and commas, you must incorporate them into your pattern.

@laverdet
Copy link

It may be worth revisiting this now that nested patterns are officially supported in the CSS specification. & selectors are required and getting a stylelint configuration which works here is cumbersome. I landed on this configuration: "selector-nested-pattern": "^((^|,)\\s*&([^,]+|[^,]*\\([^)]+\\)[^,]*))+$" but it seems unreasonable that we need to go to such lengths.

The expression is annoying because you need to catch nested commas like: &.foo, &:not(.bar, .etc). It, of course, only allows one level of parenthesis nesting. I'm not sure how deep you can go with parenthesized selectors but 1 level is the most I've seen in practice.

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: new option a new option for an existing rule good first issue is good for newcomers labels Jun 20, 2022
@jeddy3 jeddy3 changed the title selector-nested-pattern does not separate by , Add splitList: bool to selector-nested-pattern Jun 20, 2022
@jeddy3
Copy link
Member

jeddy3 commented Jun 20, 2022

@laverdet Thanks for chiming in. It does seem cumbersome, especially as we recently turned on a rule in the standard config to encourage the use of the :not(x, y) pattern.

We can add an splitList: bool (default: false) option to apply the pattern against each selector in a list, rather than the whole selector. This will avoid a breaking change to the rule.

(Prehaps there's a better name?. Prior art is the disallowInList option. Spec ref for "list".)

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

There are steps on how to add a new option in the Developer guide.

@jeddy3 jeddy3 reopened this Jun 20, 2022
@ybiquitous ybiquitous added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: new option a new option for an existing rule
Development

Successfully merging a pull request may close this issue.

5 participants