-
-
Notifications
You must be signed in to change notification settings - Fork 512
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
refactor(css_semantic): improve CSS semantic model structure and API #4908
base: main
Are you sure you want to change the base?
Conversation
// #[cfg(test)] | ||
// mod tests { | ||
// use biome_css_parser::parse_css; | ||
// use biome_css_parser::CssParserOptions; | ||
// use biome_rowan::TextRange; | ||
|
||
// #[test] | ||
// fn test_simple_ruleset() { | ||
// let parse = parse_css( | ||
// r#"p { | ||
// font-family: verdana; | ||
// font-size: 20px; | ||
// }"#, |
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.
Temporarily disable the tests. I will make another PR to split them into another module.
CodSpeed Performance ReportMerging #4908 will improve performances by 8.88%Comparing Summary
Benchmarks breakdown
|
}, | ||
range: node.text_range(), | ||
value: CssValue { node: value }, |
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.
imho: can be change to property_value
as it aligns well with property_name
?
value: CssValue { node: value }, | |
value: CssValue { node: property_value }, |
Summary
CssSyntaxNode
instead of a String so that the model has more flexibility and extensibilityTest Plan
CI should pass