diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts b/clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts --- a/clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts +++ b/clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts @@ -89,6 +89,13 @@ // highlighter being created. this.highlighter = new Highlighter(this.scopeLookupTable); this.loadCurrentTheme(); + // Event handling for handling with TextDocuments/Editors lifetimes. + vscode.window.onDidChangeVisibleTextEditors( + (editors: vscode.TextEditor[]) => + editors.forEach((e) => this.highlighter.applyHighlights( + e.document.uri.toString()))); + vscode.workspace.onDidCloseTextDocument( + (doc) => this.highlighter.removeFileHighlightings(doc.uri.toString())); } handleNotification(params: SemanticHighlightingParams) { @@ -150,12 +157,8 @@ }; 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.getVisibleTextEditorUris().forEach((fileUri) => + this.applyHighlights(fileUri)); } // Adds incremental highlightings to the current highlightings for the file @@ -171,6 +174,13 @@ this.applyHighlights(fileUri); } + // Called when a text document is closed. Removes any highlighting entries for + // the text document that was closed. + public removeFileHighlightings(fileUri: string) { + // If there exists no entry the call to delete just returns false. + this.files.delete(fileUri); + } + // Gets the uris as strings for the currently visible text editors. protected getVisibleTextEditorUris(): string[] { return vscode.window.visibleTextEditors.map((e) => @@ -180,6 +190,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(() => []); @@ -194,7 +209,11 @@ } // Applies all the highlightings currently stored for a file with fileUri. - protected applyHighlights(fileUri: string) { + public applyHighlights(fileUri: string) { + if (!this.files.has(fileUri)) + // There are no highlightings for this file, must return early or will get + // out of bounds when applying the decorations below. + return; if (!this.decorationTypes.length) // Can't apply any decorations when there is no theme loaded. return; diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts b/clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts --- a/clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts +++ b/clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts @@ -107,7 +107,7 @@ highlighter.highlight('file1', []); assert.deepEqual(highlighter.applicationUriHistory, [ 'file1' ]); highlighter.initialize(tm); - assert.deepEqual(highlighter.applicationUriHistory, [ 'file1', 'file1' ]); + assert.deepEqual(highlighter.applicationUriHistory, [ 'file1', 'file1', 'file2' ]); // Groups decorations into the scopes used. let highlightingsInLine: semanticHighlighting.SemanticHighlightingLine[] = [ { @@ -129,7 +129,7 @@ highlighter.highlight('file1', highlightingsInLine); assert.deepEqual(highlighter.applicationUriHistory, - [ 'file1', 'file1', 'file1' ]); + [ 'file1', 'file1', 'file2', 'file1' ]); assert.deepEqual(highlighter.getDecorationRanges('file1'), createHighlightingScopeRanges(highlightingsInLine)); // Keeps state separate between files. @@ -142,18 +142,24 @@ }; highlighter.highlight('file2', [ highlightingsInLine1 ]); assert.deepEqual(highlighter.applicationUriHistory, - [ 'file1', 'file1', 'file1', 'file2' ]); + [ 'file1', 'file1', 'file2', 'file1', 'file2' ]); assert.deepEqual(highlighter.getDecorationRanges('file2'), createHighlightingScopeRanges([ highlightingsInLine1 ])); // Does full colorizations. highlighter.highlight('file1', [ highlightingsInLine1 ]); assert.deepEqual(highlighter.applicationUriHistory, - [ 'file1', 'file1', 'file1', 'file2', 'file1' ]); + [ 'file1', 'file1', 'file2', '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) ])); + // Closing a text document removes all highlightings for the file and no + // other files. + highlighter.removeFileHighlightings('file1'); + assert.deepEqual(highlighter.getDecorationRanges('file1'), []); + assert.deepEqual(highlighter.getDecorationRanges('file2'), + createHighlightingScopeRanges([ highlightingsInLine1 ])); }); });