[PATCH] D67165: [clangd][vscode] Don't register notifications multiple times when clangd crashes.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
hokein planned changes to this revision.
hokein added a comment.

wait, there is still an issue.


Previously, we registered the notification everytime when the client state is
changed to running, which is unnecessary.
This patch also makes the semantic highlighting feature more
self-contained (align with the implementation of vscode builtin features)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67165

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===================================================================
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
     new vscodelc.NotificationType<SemanticHighlightingParams, void>(
         'textDocument/semanticHighlighting');
 
@@ -58,6 +58,11 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  private clangdClient: vscodelc.BaseLanguageClient;
+
+  constructor(client: vscodelc.BaseLanguageClient) {
+    this.clangdClient = client;
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
     // Extend the ClientCapabilities type and add semantic highlighting
     // capability to the object.
@@ -77,7 +82,9 @@
   }
 
   initialize(capabilities: vscodelc.ServerCapabilities,
-             documentSelector: vscodelc.DocumentSelector|undefined) {
+    documentSelector: vscodelc.DocumentSelector | undefined) {
+    // Register the handler to handle semantic highlighting notification.
+    this.clangdClient.onNotification(NotificationType, this.handleNotification);
     // The semantic highlighting capability information is in the capabilities
     // object but to access the data we must first extend the ServerCapabilities
     // type.
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===================================================================
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -110,7 +110,7 @@
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
                                                 serverOptions, clientOptions);
   const semanticHighlightingFeature =
-      new semanticHighlighting.SemanticHighlightingFeature();
+      new semanticHighlighting.SemanticHighlightingFeature(clangdClient);
   context.subscriptions.push(
       vscode.Disposable.from(semanticHighlightingFeature));
   clangdClient.registerFeature(semanticHighlightingFeature);
@@ -138,17 +138,15 @@
   context.subscriptions.push(vscode.Disposable.from(status));
   context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(
       () => { status.updateStatus(); }));
-  context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => {
-    if (newState == vscodelc.State.Running) {
-      // clangd starts or restarts after crash.
-      clangdClient.onNotification(
+
+  // The notification handler must be registered after the client is ready.
+  clangdClient.onReady().then(
+      () => {clangdClient.onNotification(
           'textDocument/clangd.fileStatus',
-          (fileStatus) => { status.onFileUpdated(fileStatus); });
-      clangdClient.onNotification(
-          semanticHighlighting.NotificationType,
-          semanticHighlightingFeature.handleNotification.bind(
-              semanticHighlightingFeature));
-    } else if (newState == vscodelc.State.Stopped) {
+          (fileStatus) => { status.onFileUpdated(fileStatus); })});
+
+  context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => {
+    if (newState == vscodelc.State.Stopped) {
       // Clear all cached statuses when clangd crashes.
       status.clear();
       semanticHighlightingFeature.dispose();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67165.218676.patch
Type: text/x-patch
Size: 3694 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190904/8ebc6712/attachment-0001.bin>