[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 05:11:07 PDT 2019

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?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list