-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Split punctuation semantic highlighting up into more tags #6238
Conversation
@@ -599,7 +599,10 @@ fn highlight_element( | |||
_ if element.parent().and_then(ast::Attr::cast).is_some() => { | |||
HighlightTag::Attribute.into() | |||
} | |||
_ => HighlightTag::Punctuation.into(), | |||
k => match punctuation_modifiers(k) { | |||
Some(modifier) => Highlight::new(HighlightTag::Punctuation) | modifier, |
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.
Hm, I think we should have Tag::Punctuation(Bracket | Brace ...)
, abusing modifiers for this at this level feels wrong.
But probably the best way to lower that to lsp in lsp_conv is indeed via modifiers: IIRC, they planned, but didn't ship inheritance for highlight tags. so we can't have punctuation.brace
, where both things are tags. But better to double-check this.
Stats: blocked on moving kinds of punctuations to tags: #6238 (comment) |
ping @Veykril ) |
Kind of forgot about this PR 😅 This should be what you had in mind I believe right? |
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.
bors r+
HlTag::Punctuation(punct) => match punct { | ||
HlPunct::Bracket => "bracket", | ||
HlPunct::Brace => "brace", | ||
HlPunct::Parenthesis => "parentheses", |
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.
Hmm.
This is the plural, I think.
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 always mix those up
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.
Oof, how do I run the HTML highlighting tests? Even with RUN_SLOW_TEST=1 cargo test
nothing fails after changing that line.
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.
Never mind, saving the changed files is a good idea 😄.
Open question would be the name of the delimiter modifiers. I chose them this was as I see them this way but from what I remember people tend to mix the names however they like. So maybe using
delimSquare
,delimCurly
,delimRound
would be better. That would also go well withangle
becomingdelimAngle
?Closes #6152