hokein added inline comments.
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:144
+// sends.
+export class Colorizer<T> {
+ private files: Map<string, Map<number, SemanticHighlightingLine>> = new
Map();
----------------
The generic `T` doesnt seem to be used?
Regarding the name, the word `colorization` is from vscode cpp-tools extension,
I think in clangd we should keep using `Hightlight` for consistency, maybe use
`Highlighter`?
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:145
+export class Colorizer<T> {
+ private files: Map<string, Map<number, SemanticHighlightingLine>> = new
Map();
+ private colorizers: Map<string, IFileColorizer> = new Map();
----------------
could you add some documentation for these fields?
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:146
+ private files: Map<string, Map<number, SemanticHighlightingLine>> = new
Map();
+ private colorizers: Map<string, IFileColorizer> = new Map();
+ private themeRuleMatcher: ThemeRuleMatcher;
----------------
It seems that, all files are sharing with duplicated implementations in this
patch, storing duplications is wasteful.
Maybe we could make the `colorize` method as a virtual method -- in the
unittest, we create a subclass of `Colorizer` and override the `colorize`
method.
================
Comment at:
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:164
+ public setFileLines(uriString: string, tokens: SemanticHighlightingLine[]) {
+ // Patch in the new highlightings to the highlightings cache. If this is
the
+ // first time the file should be highlighted a new colorizer and a file
----------------
we are patching the incremental diff to get a "global" highlighting for the
whole file, but it maybe not correct -- clangd doesn't emit diff outside of the
max line number of the file, if we delete a few lines at the end of the file,
then we will keep the removed lines in the map.
I think this is a current limitation, could you check with the current behavior
of vscode if we pass a out-of-range to the decoration API?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66219/new/
https://reviews.llvm.org/D66219
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
