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

Validate selectors consistently. Fixes #3130 #4483

Merged

Conversation

dom111
Copy link
Contributor

@dom111 dom111 commented Dec 13, 2019

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

Closes #3130.

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

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.

Thanks for fixing this!

I think we can use isStandardSyntaxSelector in isStandardSyntaxRule, because for standard syntax rule selector also should be in standard syntax.

This way we won't need checks like:

if (!isStandardSyntaxRule(rule) || !isStandardSyntaxSelector(rule.selector)) {

We'll keep more simple:

if (!isStandardSyntaxRule(rule)) {

@dom111
Copy link
Contributor Author

dom111 commented Dec 22, 2019

@hudochenkov

Thanks for fixing this!

No problem, happy to help!

I think we can use isStandardSyntaxSelector in isStandardSyntaxRule, because for standard syntax rule selector also should be in standard syntax.

This makes sense and I've stripped it out of most of the places that make sense. There are a few separate calls left but from what I gathered I think they're separate selector validations around nested and surrounding selectors.

Thanks!

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.

Thank you!

@dom111 dom111 merged commit 0f04539 into stylelint:master Dec 23, 2019
@dom111 dom111 deleted the enhancement/3130-consistently-check-selectors branch December 23, 2019 08:03
@hudochenkov
Copy link
Member

Please, as a new contributor, don't merge PRs just yet. Learn our processes, and observe.

@hudochenkov
Copy link
Member

  • Fixed: consistently check that selectors are standard before passing to the parser (#4483).

@dom111
Copy link
Contributor Author

dom111 commented Dec 23, 2019

@hudochenkov Apologies, I didn't see the option to merge before, so when I saw the option existed I switched to following my usual process, (although the fact it's merging to master rather that an integration branch...) I'll refrain from merging in the future though, was just trying to save some work! :)

@hudochenkov
Copy link
Member

No worries :)

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.

Consistently check that selectors are standard before passing to the parser
3 participants