-
-
Notifications
You must be signed in to change notification settings - Fork 503
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(css_semantic): resolve nested selectors #4658
base: main
Are you sure you want to change the base?
Conversation
for name in resolved_selectors.iter() { | ||
current_rule.selectors.push(Selector { | ||
name: name.to_string(), |
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.
for name in resolved_selectors.iter() { | |
current_rule.selectors.push(Selector { | |
name: name.to_string(), | |
for selector_name in resolved_selectors.iter() { | |
current_rule.selectors.push(Selector { | |
name: selector_name.to_string(), |
name
is somewhat ambiguous, and selector_name
would improve readability IMHO
@@ -173,3 +185,23 @@ impl SemanticModelBuilder { | |||
} | |||
} | |||
} | |||
|
|||
fn resolve_selector(current: &str, parent: &[Selector]) -> Vec<String> { |
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.
fn resolve_selector(current: &str, parent: &[Selector]) -> Vec<String> { | |
fn resolve_selector(current: &str, parents: &[Selector]) -> Vec<String> { |
nit: parents
is more accurate since it's a slice of selectors?
let mut resolved = Vec::new(); | ||
|
||
if parent.is_empty() { | ||
return vec![current.to_string()]; | ||
} | ||
|
||
for parent in parent { | ||
let parent = parent.name.to_string(); | ||
if current.contains('&') { | ||
let current = current.replace('&', &parent); | ||
resolved.push(current); | ||
} else { | ||
resolved.push(format!("{} {}", parent, current)); | ||
} | ||
} | ||
|
||
resolved |
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 mut resolved = Vec::new(); | |
if parent.is_empty() { | |
return vec![current.to_string()]; | |
} | |
for parent in parent { | |
let parent = parent.name.to_string(); | |
if current.contains('&') { | |
let current = current.replace('&', &parent); | |
resolved.push(current); | |
} else { | |
resolved.push(format!("{} {}", parent, current)); | |
} | |
} | |
resolved | |
if parents.is_empty() { | |
return vec![current.to_string()]; | |
} | |
parents | |
.iter() | |
.map(|parent| { | |
let parent_name = &parent.name; | |
if current.contains('&') { | |
current.replace('&', parent_name) | |
} else { | |
format!("{} {}", parent_name, current) | |
} | |
}) | |
.collect() |
let current_rule = self.rules_by_id.get_mut(current_rule_id).unwrap(); | ||
for name in resolved_selectors.iter() { | ||
current_rule.selectors.push(Selector { | ||
name: name.to_string(), |
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.
We already have a String
, we shouldn't create a new one
@@ -173,3 +185,23 @@ impl SemanticModelBuilder { | |||
} | |||
} | |||
} | |||
|
|||
fn resolve_selector(current: &str, parent: &[Selector]) -> Vec<String> { |
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.
It's possible that I am late to the party, so I am sorry if this questions come too late.
Is there a particular reason we need to track String
inside the semantic model? I wonder if we can avoid that and store only TextRange
and syntax nodes (syntax nodes are cheaper than strings) inside the model.
Summary
The actual result would be like
Example
Resolves to:
Resolves to:
Resolves to:
Resolves to:
Test Plan
Added tests