jvikstrom added inline comments.
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:135
+ // Update the themeRuleMatcher that is used when highlighting. Also triggers
a
+ // recolorization for all current highlighters. Safe to call multiple times.
+ public initialize(themeRuleMatcher: ThemeRuleMatcher) {
----------------
hokein wrote:
> nit: the comment is stale now. I believe this function is only called
> whenever we reload a theme or at the first launch of the extension.
It's should only called when a theme is loaded (and we happen to load a theme
when we initialize, but it's async).
Don't see why the comment is stale though, it seems to describe what it does to
me.
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:139
+ const options: vscode.DecorationRenderOptions = {
+ color : themeRuleMatcher.getBestThemeRule(scopes[0]).foreground,
+ // If the rangeBehavior is set to Open in any direction the
----------------
hokein wrote:
> just want to confirm: if we fail to find a matched theme rule, we return an
> empty decoration. I think vscode just doesn't do anything on an empty color?
vscode indeed does not do anything on empty colors.
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts:94
+ let line = [
+ {character : 1, length : 2, scopeIndex : 1},
+ {character : 5, length : 2, scopeIndex : 1},
----------------
hokein wrote:
> I believe the test code is correct, but the code here/below is complex and
> hard to follow. I think we need make the code more readable/understandable.
>
> some suggestions:
>
> - the line number is implicitly indicated by the index of the array, I think
> we can add one more field `line` to the structure (call HighlightingTokens).
> - and creates some helper functions (with proper names) that take the
> `HighlightingTokens` as parameter and generate the data you need e.g.
> `SemanticHighlightingLine[]`, `vscode.Range`.
>
Started exporting the `SemanticHighlightingLine` interface and use that format
in the tests to not create more interfaces (also means we don't need a helper
for creating the SemanticHighlightingLine).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66219/new/
https://reviews.llvm.org/D66219
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
