hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:37 +// experimental semantic highlighting. +export class Feature implements vscodelc.StaticFeature { + scopes: string[]; ---------------- nit: name it `SemanticHighlightingFeature`. ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:38 +export class Feature implements vscodelc.StaticFeature { + scopes: string[]; + fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) { ---------------- I believe the type of lookup table returned from clangd is a string[][]. I'd name it `scopeLookupTable` ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:62 + + handleNotification(params: HighlightingInformation) { + const tokenLines = params.lines.map((line): HighlightingLine => { ---------------- I think the params here is defined as `SemanticHighlightingParams` from the LSP proposal, could we use the names as defined in the LSP for these structures (e.g. `SemanticHighlightingParams`, `SemanticHighlightingInformation`)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65998/new/ https://reviews.llvm.org/D65998 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits