-
-
Notifications
You must be signed in to change notification settings - Fork 475
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_analyzer): noCssEmptyBlock #2513
Conversation
CodSpeed Performance ReportMerging #2513 will improve performances by 6.38%Comparing Summary
Benchmarks breakdown
|
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! I think the option should be revisited because it's very hard to understand what it does... and why ["comments"]
?
use serde::{Deserialize, Serialize}; | ||
|
||
declare_rule! { | ||
/// Disallow css empty blocks. |
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.
/// Disallow css empty blocks. | |
/// Disallow CSS empty blocks. |
declare_rule! { | ||
/// Disallow css empty blocks. | ||
/// | ||
/// This rule disallows empty block. |
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.
/// This rule disallows empty block. |
Repetition of the previous sentence.
/// { | ||
/// "noCssEmptyBlock": { | ||
/// "options": { | ||
/// "ignore": ["comments"] |
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.
This option - coming from Styelint- isn't very... good. If by adding this option we allow comments in blocks, then why not "options": { "allowComments": true }
?
Also, it's really hard to understand what's the default behaviour of the rule, which we should explain in the documentation.
rule_category!(), | ||
span, | ||
markup! { | ||
"Unexpected empty block is not allowed" |
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.
"Unexpected empty block is not allowed" | |
"Empty blocks aren't allowed." |
a7d229e
to
d1d4206
Compare
Summary
close #2512
Implement block-no-empty
Test Plan
added tests and snapshots