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

Added: selector-max-combinators rule. #2658

Merged
merged 1 commit into from
Jun 26, 2017
Merged

Conversation

alexander-akait
Copy link
Member

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

#2528

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

@@ -163,6 +163,7 @@ Here are all the rules within stylelint, grouped by the [*thing*](http://apps.wo
- [`selector-descendant-combinator-no-non-space`](../../lib/rules/selector-descendant-combinator-no-non-space/README.md): Disallow non-space characters for descendant combinators of selectors.
- [`selector-id-pattern`](../../lib/rules/selector-id-pattern/README.md): Specify a pattern for id selectors.
- [`selector-max-class`](../../lib/rules/selector-max-class/README.md): Limit the number of classes in a selector.
- [`selector-max-combinators`](../../lib/rules/selector-max-combinators/README.md): Limit the number of combinators selectors in a selector.
Copy link
Member

Choose a reason for hiding this comment

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

"Limit the number of combinators in a selector."

Copy link
Member Author

Choose a reason for hiding this comment

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

@jeddy3 done

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.

@evilebottnawi Thanks for this.

I've made some doc and message requests.

Remember to update the example config and rules list docs in this PR.

I'm out of time and yet to look at the tests I'm afraid.

@@ -0,0 +1,61 @@
# selector-max-combinators

Limit the number of combinators selectors in a selector.
Copy link
Member

Choose a reason for hiding this comment

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

"Limit the number of combinators in a selector."

```

```css
a, b {}
Copy link
Member

Choose a reason for hiding this comment

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

Use a b ~ c here for parity with positive patterns.

```

```css
a.foo {}
Copy link
Member

Choose a reason for hiding this comment

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

Use the following instead here:

a b {
  & ~ c {}
}

```

```css
/* each selector in a selector list is evaluated separately */
Copy link
Member

Choose a reason for hiding this comment

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

Use the following instead:

a b,
c > d {}

const ruleName = "selector-max-combinators"

const messages = ruleMessages(ruleName, {
expected: (selector, max) => `Expected "${selector}" to have no more than ${max} combinators ${max === 1 ? "selector" : "selectors"}`,
Copy link
Member

Choose a reason for hiding this comment

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

expected: (selector, max) => `Expected "${selector}" to have no more than ${max}  ${max === 1 ? "combinator" : "combinators"}`,

@alexander-akait
Copy link
Member Author

@jeddy3 done, about tests: all tested was migrate from basic rules and add more tests, in theory all should be fine. Btw, if someone found error we just fix it and release patch version.

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Looks good.

Few small changes required: in tests descriptions lots of “max id selectors”, which are from other rule :)

description: "pseudo selectors",
}, {
code: "foo bar, \nbaz quux {}",
description: "multiple selectors: exactly max id selectors",
Copy link
Member

Choose a reason for hiding this comment

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

“multiple selectors: fewer then max combinators selectors”

@jeddy3
Copy link
Member

jeddy3 commented Jun 26, 2017

@hudochenkov Changes made.

@jeddy3 jeddy3 mentioned this pull request Jun 26, 2017
5 tasks
@jeddy3 jeddy3 merged commit 56f9fca into master Jun 26, 2017
@jeddy3 jeddy3 deleted the selector-max-combinators branch June 26, 2017 10:56
@jeddy3
Copy link
Member

jeddy3 commented Jun 26, 2017

  • Added: selector-max-combinators rule (#2658).

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