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 auto fix for selector-pseudo-class-case rule #3671

Merged
merged 8 commits into from Sep 24, 2018

Conversation

3 participants
@Bilie
Contributor

Bilie commented Sep 15, 2018

Closes #3661

No, it's self explanatory.

@ota-meshi

Thank you for this pull request!
I have some change requests.

Show resolved Hide resolved lib/rules/selector-pseudo-class-case/__tests__/index.js
@@ -61,6 +61,11 @@ const rule = function(expectation) {
return;
}
if (context.fix) {
rule.selector = rule.selector.replace(pseudo, expectedPseudo);

This comment has been minimized.

@ota-meshi

ota-meshi Sep 16, 2018

Member

You can also directly replace the value of pseudoNode, and receive it as the return value of parseSelector.

Bilie added some commits Sep 20, 2018

@@ -61,6 +61,19 @@ const rule = function(expectation) {
return;
}
if (context.fix) {
if (rule.raws.selector) {
rule.raws.selector.raw = rule.raws.selector.raw.replace(

This comment has been minimized.

@ota-meshi

ota-meshi Sep 20, 2018

Member

I found a problem with the CSS as follow.

a::FIRST-LETTER, a:FIRST {color: pink;}

The result of fixed this is as follow.

a::first-LETTER, a:FIRST {color: pink;}

To improve this, I think we need to change this as follows.

pseudoNode.value = expectedPseudo;

Then we process the result of parseSelector as follows.

const fixedSelector = parseSelector(rule.raws.selector ? rule.raws.selector.raw : rule.selector, ...
//...

if (context.fix) {
  if (rule.raws.selector) {
    rule.raws.selector.raw = fixedSelector
  } else {
    rule.selector = fixedSelector;
  }
}

This comment has been minimized.

@Bilie

Bilie Sep 20, 2018

Contributor

Hi, thank you for spotting this case! I have updated the PR

const fixedSelector = selector.replace(pseudo, expectedPseudo);
if (rule.raws.selector) {
rule.raws.selector.raw = rule.raws.selector.raw.replace(

This comment has been minimized.

@ota-meshi

ota-meshi Sep 21, 2018

Member

The following css will not be autofixed.

.foo > /* comment */ a:Hover { color: pink; }

I think that this problem will not occur with the method I proposed before.
Do you have reason to process with replace?

I'm sorry if you can not understand, because I am not good at English.

This comment has been minimized.

@Bilie

Bilie Sep 21, 2018

Contributor

Hi, certainly not your English! :) I am just still trying to understand the postcss api, all the possible test cases, and what is the best way to solve a certain issue. Thank you for your help!

@ota-meshi

LGTM!!

@jeddy3

jeddy3 approved these changes Sep 24, 2018

@jeddy3 jeddy3 merged commit fbe3429 into stylelint:master Sep 24, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 96.404%
Details
@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 24, 2018

  • Added: selector-pseudo-class-case autofix (#3671).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment