-
-
Notifications
You must be signed in to change notification settings - Fork 938
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
Fix false positives for at-charset and single quotes in string-quotes #2788 #2902
Conversation
lib/rules/string-quotes/README.md
Outdated
@@ -10,6 +10,8 @@ a[id="foo"] { content: "x"; } | |||
|
|||
Quotes within comments are ignored. | |||
|
|||
Also, the quotes in a `charset` @-rule are ignored as using single quotes in this context is incorrect according the [CSS specification](https://www.w3.org/TR/CSS2/syndata.html#x57). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this line to below the quotes in comments code underneath this sentence
Update the verbiage to Single quotes in a charset @-rule are ignored as using single quotes in this context is incorrect according the CSS specification.
Maybe see what adding a code example for this also looks like, it might look weird or add confusion 🤔
}, | ||
{ | ||
code: '@charset "utf-8"', | ||
description: "ignore @charset rules" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this same test to the "double quotes" test around line #119 as a safe guard of sorts please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added below ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, though you added it to the SCSS tests at lines 160-163, which won't hurt either, can you add it after line 118 please :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops. Yes added there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!
|
#2788
No, self-explanatory