Merge semantic and syntax highlighting#1401
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1401 +/- ##
=========================================
- Coverage 6.16% 6.16% -0.01%
=========================================
Files 122 122
Lines 49341 49355 +14
=========================================
Hits 3041 3041
- Misses 46300 46314 +14
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
lapce-data/src/document.rs
Outdated
|
|
||
| match (semantic_styles, syntactic_styles) { | ||
| (Some(semantic), Some(syntax)) => { | ||
| Some(Arc::new(syntax.merge(semantic, |a, b| match b { |
There was a problem hiding this comment.
I think this merge needs to be happening when semantic styles and tree sitter styles get updated.
And we can just replace semantic styles with the merged styles.
There was a problem hiding this comment.
Are those changes alright for now?
If we notice perf issues, we can use https://github.com/xi-editor/xi-editor/blob/master/rust/rope/src/spans.rs#L193-L287
As a more efficient solution (that way we only build the spans once instead of 3x. The reason it would be a pain to do now is that the spans have to be added sequentially which isn't the easiest to do
5500360 to
5c80440
Compare
lapce-data/src/document.rs
Outdated
|
|
||
| if let Some(syntactic_styles) = syntactic_styles { | ||
| for (interval, style) in syntactic_styles.iter() { | ||
| syntactic_styles_spans |
There was a problem hiding this comment.
Any reason you want to rebuild the styles spans here?
syntactic_styles is Spans<Style> already
There was a problem hiding this comment.
thanks, didn't realise that.
No reason... just an oversight
5c80440 to
9fe1554
Compare
semantic token highlighting from the lsp does not return all the highlights, instead we can combine it with the highlighting from tree sitter for a more complete experience
Before:
After: