jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
Disposes of the vscode listeners when clangd crashes and reuses the old
highlighter when it restarts. The reason for reusing the highlighter is because
this way the highlightings will not disappear as we won't have to dispose of
them.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66743
Files:
clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
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
@@ -107,7 +107,8 @@
highlighter.highlight('file1', []);
assert.deepEqual(highlighter.applicationUriHistory, [ 'file1' ]);
highlighter.initialize(tm);
- assert.deepEqual(highlighter.applicationUriHistory, [ 'file1', 'file1', 'file2' ]);
+ assert.deepEqual(highlighter.applicationUriHistory,
+ [ 'file1', 'file1', 'file2' ]);
// Groups decorations into the scopes used.
let highlightingsInLine: semanticHighlighting.SemanticHighlightingLine[] = [
{
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
@@ -56,6 +56,8 @@
scopeLookupTable: string[][];
// The object that applies the highlightings clangd sends.
highlighter: Highlighter;
+ // Any disposables that should be cleaned up when clangd crashes.
+ private disposables: vscode.Disposable[] = [];
fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
// Extend the ClientCapabilities type and add semantic highlighting
// capability to the object.
@@ -87,15 +89,19 @@
// 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);
+ if (!this.highlighter)
+ // If there already is a highlighter existing there is no need to create a
+ // new one, just reset and keep using it.
+ this.highlighter = new Highlighter(this.scopeLookupTable);
+ this.highlighter.clear();
this.loadCurrentTheme();
// Event handling for handling with TextDocuments/Editors lifetimes.
- vscode.window.onDidChangeVisibleTextEditors(
+ this.disposables.push(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()));
+ e.document.uri.toString()))));
+ this.disposables.push(vscode.workspace.onDidCloseTextDocument(
+ (doc) => this.highlighter.removeFileHighlightings(doc.uri.toString())));
}
handleNotification(params: SemanticHighlightingParams) {
@@ -103,6 +109,12 @@
(line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
this.highlighter.highlight(params.textDocument.uri, lines);
}
+ // Disposes of any resources that are not reused if clangd crashes and
+ // restarts.
+ public crashDispose() {
+ this.disposables.forEach((d) => d.dispose());
+ this.disposables = [];
+ }
}
// Converts a string of base64 encoded tokens into the corresponding array of
@@ -138,6 +150,7 @@
constructor(scopeLookupTable: string[][]) {
this.scopeLookupTable = scopeLookupTable;
}
+ public clear() { this.files.clear(); }
// This function must be called at least once or no highlightings will be
// done. Sets the theme that is used when highlighting. Also triggers a
// recolorization for all current highlighters. Should be called whenever the
@@ -174,6 +187,27 @@
this.applyHighlights(fileUri);
}
+ // Applies all the highlightings currently stored for a file with fileUri.
+ 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;
+ // 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) => {
+ if (e.document.uri.toString() !== fileUri)
+ return;
+ this.decorationTypes.forEach((d, i) => e.setDecorations(d, ranges[i]));
+ });
+ }
+
// Called when a text document is closed. Removes any highlighting entries for
// the text document that was closed.
public removeFileHighlightings(fileUri: string) {
@@ -207,27 +241,6 @@
});
return decorations;
}
-
- // Applies all the highlightings currently stored for a file with fileUri.
- 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;
- // 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) => {
- if (e.document.uri.toString() !== fileUri)
- return;
- this.decorationTypes.forEach((d, i) => e.setDecorations(d, ranges[i]));
- });
- }
}
// A rule for how to color TextMate scopes.
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===================================================================
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -112,14 +112,6 @@
const semanticHighlightingFeature =
new semanticHighlighting.SemanticHighlightingFeature();
clangdClient.registerFeature(semanticHighlightingFeature);
- // The notification handler must be registered after the client is ready or
- // the client will crash.
- clangdClient.onReady().then(
- () => clangdClient.onNotification(
- semanticHighlighting.NotificationType,
- semanticHighlightingFeature.handleNotification.bind(
- semanticHighlightingFeature)));
-
console.log('Clang Language Server is now active!');
context.subscriptions.push(clangdClient.start());
context.subscriptions.push(vscode.commands.registerCommand(
@@ -149,9 +141,14 @@
clangdClient.onNotification(
'textDocument/clangd.fileStatus',
(fileStatus) => { status.onFileUpdated(fileStatus); });
+ clangdClient.onNotification(
+ semanticHighlighting.NotificationType,
+ semanticHighlightingFeature.handleNotification.bind(
+ semanticHighlightingFeature));
} else if (newState == vscodelc.State.Stopped) {
// Clear all cached statuses when clangd crashes.
status.clear();
+ semanticHighlightingFeature.crashDispose();
}
})
// An empty place holder for the activate command, otherwise we'll get an
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
