[PATCH] D66735: [clangd] Handling text editor/document lifetimes in vscode extension.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Just reapplies highlightings for all files when visible text editors change. Could find the correct text editor that should be reapplied but going for a simple implementation instead.
Removes the associated highlighting entry from the Colorizer when a text document is closed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66735

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


Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===================================================================
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -155,5 +155,19 @@
         highlighter.getDecorationRanges('file1'),
         createHighlightingScopeRanges(
             [ highlightingsInLine1, ...highlightingsInLine.slice(1) ]));
+    // Opening a text editor reapplies all highlightings.
+    highlighter.onDidChangeVisibleTextEditors();
+    assert.deepEqual(
+        highlighter.applicationUriHistory,
+        [ 'file1', 'file1', 'file1', 'file2', 'file1', 'file1', 'file2' ]);
+    assert.deepEqual(
+        highlighter.getDecorationRanges('file1'),
+        createHighlightingScopeRanges(
+            [ highlightingsInLine1, ...highlightingsInLine.slice(1) ]));
+    assert.deepEqual(highlighter.getDecorationRanges('file2'),
+                     createHighlightingScopeRanges([ highlightingsInLine1 ]));
+    // Closing a text document removes all highlightings for the file.
+    highlighter.onDidCloseTextDocument('file1');
+    assert.deepEqual(highlighter.getDecorationRanges('file1'), []);
   });
 });
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
@@ -89,6 +89,11 @@
     // highlighter being created.
     this.highlighter = new Highlighter(this.scopeLookupTable);
     this.loadCurrentTheme();
+    // Event handling for handling with TextDocuments/Editors lifetimes.
+    vscode.window.onDidChangeVisibleTextEditors(
+        () => this.highlighter.onDidChangeVisibleTextEditors());
+    vscode.workspace.onDidCloseTextDocument(
+        (doc) => this.highlighter.onDidCloseTextDocument(doc.uri.toString()));
   }
 
   handleNotification(params: SemanticHighlightingParams) {
@@ -150,12 +155,7 @@
       };
       return vscode.window.createTextEditorDecorationType(options);
     });
-    this.getVisibleTextEditorUris().forEach((fileUri) => {
-      // A TextEditor might not be a cpp file. So we must check we have
-      // highlightings for the file before applying them.
-      if (this.files.has(fileUri))
-        this.applyHighlights(fileUri);
-    })
+    this.reapplyAllHighlightings();
   }
 
   // Adds incremental highlightings to the current highlightings for the file
@@ -171,6 +171,25 @@
     this.applyHighlights(fileUri);
   }
 
+  // Called when a text document is closed. Removes any highlighting entries for
+  // the text document that was closed.
+  public onDidCloseTextDocument(fileUri: string) {
+    // If there exists no entry the call to delete just returns false.
+    this.files.delete(fileUri);
+  }
+
+  // Called when the currently visible text editors have changed. Reapplies
+  // highlightings to all visible text editors that highlightings exist for.
+  public onDidChangeVisibleTextEditors() { this.reapplyAllHighlightings(); }
+
+  protected reapplyAllHighlightings() {
+    this.getVisibleTextEditorUris().forEach((fileUri) => {
+      // A TextEditor might not be a cpp file. So we must check we have
+      // highlightings for the file before applying them.
+      if (this.files.has(fileUri))
+        this.applyHighlights(fileUri);
+    });
+  }
   // Gets the uris as strings for the currently visible text editors.
   protected getVisibleTextEditorUris(): string[] {
     return vscode.window.visibleTextEditors.map((e) =>
@@ -180,6 +199,11 @@
   // Returns the ranges that should be used when decorating. Index i in the
   // range array has the decoration type at index i of this.decorationTypes.
   protected getDecorationRanges(fileUri: string): vscode.Range[][] {
+    if (!this.files.has(fileUri))
+      // this.files should always have an entry for fileUri if we are here. But
+      // if there isn't one we don't want to crash the extension. This is also
+      // useful for tests.
+      return [];
     const lines: SemanticHighlightingLine[] =
         Array.from(this.files.get(fileUri).values());
     const decorations: vscode.Range[][] = this.decorationTypes.map(() => []);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66735.217122.patch
Type: text/x-patch
Size: 4410 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190826/b7ce684c/attachment.bin>