[clang-tools-extra] r369911 - [clangd] Handling text editor/document lifetimes in vscode extension.
Johan Vikstrom via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 26 06:56:45 PDT 2019
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 ]));
});
});
More information about the cfe-commits
mailing list