hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:8 +// The information clangd sends when highlightings should be updated. +interface SemanticHighlightingParams { + // The information about the text document where these highlightings should be ---------------- sorry for not being clear in the previous comment, I think we should mirror definitions from the LSP [proposal](https://github.com/microsoft/vscode-languageserver-node/pull/367/files#diff-ac2186ae4b8ba5cc9f17deebc42c9e28R13), the same to the `SemanticHighlightingInformation`. You may need to add a `vscode-languageserver-types` dependency. ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:32 + +export const NotificationType = new vscodelc.NotificationType<{}, void>( + 'textDocument/semanticHighlighting'); ---------------- I think here we should use `SemanticHighlightingParams` in the generic type parameter list ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:38 +export class SemanticHighlightingFeature implements vscodelc.StaticFeature { + scopeLookupTable: string[][]; + fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) { ---------------- could you add a comment on this? ================ Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:63 + handleNotification(params: SemanticHighlightingParams) { + const tokenLines = + params.lines.map((line): SemanticHighlightingInformation => { ---------------- this seems doesn't do anything, can we defer it to a follow-up patch? 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