Author: jvikstrom Date: Mon Aug 26 06:56:45 2019 New Revision: 369911 URL: http://llvm.org/viewvc/llvm-project?rev=369911&view=rev Log: [clangd] Handling text editor/document lifetimes in vscode extension.
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. Reviewers: hokein, ilya-biryukov Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D66735 Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts?rev=369911&r1=369910&r2=369911&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts (original) +++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts Mon Aug 26 06:56:45 2019 @@ -89,6 +89,13 @@ export class SemanticHighlightingFeature // 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 @@ export class Highlighter { }; 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 @@ export class Highlighter { 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 @@ export class Highlighter { // 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 @@ export class Highlighter { } // 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; Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts?rev=369911&r1=369910&r2=369911&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts (original) +++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts Mon Aug 26 06:56:45 2019 @@ -107,7 +107,7 @@ suite('SemanticHighlighting Tests', () = 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 @@ suite('SemanticHighlighting Tests', () = 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 @@ suite('SemanticHighlighting Tests', () = }; 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 ])); }); }); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
