-
-
Notifications
You must be signed in to change notification settings - Fork 523
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
feat(biome_css_analyze): implement noIrregularWhitespaceCss #3428
feat(biome_css_analyze): implement noIrregularWhitespaceCss #3428
Conversation
CodSpeed Performance ReportMerging #3428 will degrade performances by 38.77%Comparing Summary
Benchmarks breakdown
|
/// ``` | ||
/// | ||
pub NoIrregularWhitespaceCss { | ||
version: "1.8.0", |
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.
version: "1.8.0", | |
version: "next", |
version: "1.8.0", | ||
name: "noIrregularWhitespaceCss", | ||
language: "css", | ||
recommended: false, |
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.
recommended: false, | |
recommended: true, |
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.
I don't think we should recommend it. It's a slow rule, and a niche one.
// | ||
// Read our guidelines to write great diagnostics: | ||
// https://docs.rs/biome_analyze/latest/biome_analyze/#what-a-rule-should-say-to-the-user | ||
// |
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.
Please remove the comment
/// } | ||
/// ``` | ||
/// | ||
pub NoIrregularWhitespaceCss { |
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.
CSS
isn't necessary. One rule can use the same name in multiple languages in Biome.
pub NoIrregularWhitespaceCss { | |
pub NoIrregularWhitespace { |
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.
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.
No, problem. Keep the old name and I will handle that once this rule is merged
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.
Thank you! Could you add CHANGELOG
and run just gen-lint
again?
'\u{c}', '\u{b}', '\u{85}', '\u{feff}', '\u{a0}', '\u{1680}', '\u{180e}', '\u{2000}', | ||
'\u{2001}', '\u{2002}', '\u{2003}', '\u{2004}', '\u{2005}', '\u{2006}', '\u{2007}', '\u{2008}', | ||
'\u{2009}', '\u{200a}', '\u{200b}', '\u{202f}', '\u{205f}', '\u{3000}', | ||
]; |
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.
@DerTimonius are you still interested in this PR? |
@ematipico yes, I was wondering what is still missing |
Just fix the conflicts and I'll merge it |
@ematipico thanks, resolved the conflicts 🙂 |
Summary
This PR supercedes #3362 (there were some collisions between the css and the js rule and it was harder to fix than anticipated, so I started over) and implements the
no-irregular-whitespace
rule from stylelint.My original implementation only checked for selectors, now this rule checks for complete rule blocks, regardless of type of rule (import rules, container rules, simple rule, you name it).
I was able to reuse most of the logic used in #3333 with some changes.
Test Plan
I tested some different @-rules, used the whitespace in the selector and in the rule itself. I also used all characters that are declared irregular.
closes #3264