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

[jvikstrom]

Summary

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.

[hokein]

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

94

I think we can do it like vscode.window.onDidChangeVisibleTextEditors((TextEditor[] editors) => { // call applyHighlight for each text editor }).

176

the name is a bit weird onDidCloseTextDocument in the Highlighter, I would name it removeFileHighlights

[jvikstrom]

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

94

There might be TextEditors that are not a c++ file/haven't gotten highlightings yet.

So we'd still need to do the this.files.has(fileUri) check which we also do in the initialize function.

Maybe I should just expose the reapplyAllHighlightings function and call that one instead?

[hokein]

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

94

if the file is not a c++, the current implementation of applyHighlights is tolerant with that I think, it doesn't do anything (we get an empty decoration ranges).

I think we can do the this.files.has(fileUri) check in applyHighlights.

[jvikstrom]

Address comments.

[jvikstrom]

Fixed changes that weren't supposed to be made.

[hokein]

I think there is one more case -- we need to cleanup the highlighting cache if clangd crashes, the extension will automatically restart clangd up to 5 times if it sees clangd crashes, you can see how filestatus handles it.

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

174

I think remove all highlightings for the file with the given fileUri. is enough.

221

nit: the trailing If?

222

could you move this method after highlight?

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

initialize(capabilities: vscodelc.ServerCapabilities,
     if (!serverCapabilities.semanticHighlighting)
       return;
     this.scopeLookupTable = serverCapabilities.semanticHighlighting.scopes;
     // Important that highlighter is created before the theme is loading as
     // otherwise it could try to update the themeRuleMatcher without the
     // 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) {
     const lines: SemanticHighlightingLine[] = params.lines.map(
         (line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
     this.highlighter.highlight(params.textDocument.uri, lines);
   }
 }
this.decorationTypes = this.scopeLookupTable.map((scopes) => {
         // empty decorations.
         color : themeRuleMatcher.getBestThemeRule(scopes[0]).foreground,
         // If the rangeBehavior is set to Open in any direction the
         // highlighting becomes weird in certain cases.
         rangeBehavior : vscode.DecorationRangeBehavior.ClosedClosed,
       };
       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
   // with fileUri. Also applies the highlightings to any associated
   // TextEditor(s).
   public highlight(fileUri: string,
                    highlightingLines: SemanticHighlightingLine[]) {
     if (!this.files.has(fileUri)) {
       this.files.set(fileUri, new Map());
     }
     const fileHighlightings = this.files.get(fileUri);
     highlightingLines.forEach((line) => fileHighlightings.set(line.line, line));
     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) =>
                                                     e.document.uri.toString());
   }
 
   // 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(() => []);
     lines.forEach((line) => {
       line.tokens.forEach((token) => {
         decorations[token.scopeIndex].push(new vscode.Range(
             new vscode.Position(line.line, token.character),
             new vscode.Position(line.line, token.character + token.length)));
       });
     });
     return decorations;
   }
 
   // Applies all the highlightings currently stored for a file with fileUri.
   protected applyHighlights(fileUri: string) {
     if (!this.decorationTypes.length)
       // Can't apply any decorations when there is no theme loaded.
       return;
     // This must always do a full re-highlighting due to the fact that
     // TextEditorDecorationType are very expensive to create (which makes
     // incremental updates infeasible). For this reason one
     // TextEditorDecorationType is used per scope.
     const ranges = this.getDecorationRanges(fileUri);
     vscode.window.visibleTextEditors.forEach((e) => {

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

test('Colorizer groups decorations correctly', async () => {
     assert.deepEqual(highlighter.applicationUriHistory,
                      [ 'file1', 'file1', 'file1', 'file2', 'file1' ]);
     // After the incremental update to line 1, the old highlightings at line 1
     // will no longer exist in the array.
     assert.deepEqual(
         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'), []);
   });
 });