hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. hokein planned changes to this revision. hokein added a comment.
wait, there is still an issue. Previously, we registered the notification everytime when the client state is changed to running, which is unnecessary. This patch also makes the semantic highlighting feature more self-contained (align with the implementation of vscode builtin features) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D67165 Files: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts 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 @@ -44,7 +44,7 @@ // Language server push notification providing the semantic highlighting // information for a text document. -export const NotificationType = +const NotificationType = new vscodelc.NotificationType<SemanticHighlightingParams, void>( 'textDocument/semanticHighlighting'); @@ -58,6 +58,11 @@ highlighter: Highlighter; // Any disposables that should be cleaned up when clangd crashes. private subscriptions: vscode.Disposable[] = []; + private clangdClient: vscodelc.BaseLanguageClient; + + constructor(client: vscodelc.BaseLanguageClient) { + this.clangdClient = client; + } fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) { // Extend the ClientCapabilities type and add semantic highlighting // capability to the object. @@ -77,7 +82,9 @@ } initialize(capabilities: vscodelc.ServerCapabilities, - documentSelector: vscodelc.DocumentSelector|undefined) { + documentSelector: vscodelc.DocumentSelector | undefined) { + // Register the handler to handle semantic highlighting notification. + this.clangdClient.onNotification(NotificationType, this.handleNotification); // The semantic highlighting capability information is in the capabilities // object but to access the data we must first extend the ServerCapabilities // type. 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 @@ -110,7 +110,7 @@ const clangdClient = new ClangdLanguageClient('Clang Language Server', serverOptions, clientOptions); const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature(clangdClient); context.subscriptions.push( vscode.Disposable.from(semanticHighlightingFeature)); clangdClient.registerFeature(semanticHighlightingFeature); @@ -138,17 +138,15 @@ context.subscriptions.push(vscode.Disposable.from(status)); context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor( () => { status.updateStatus(); })); - context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => { - if (newState == vscodelc.State.Running) { - // clangd starts or restarts after crash. - clangdClient.onNotification( + + // The notification handler must be registered after the client is ready. + clangdClient.onReady().then( + () => {clangdClient.onNotification( 'textDocument/clangd.fileStatus', - (fileStatus) => { status.onFileUpdated(fileStatus); }); - clangdClient.onNotification( - semanticHighlighting.NotificationType, - semanticHighlightingFeature.handleNotification.bind( - semanticHighlightingFeature)); - } else if (newState == vscodelc.State.Stopped) { + (fileStatus) => { status.onFileUpdated(fileStatus); })}); + + context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => { + if (newState == vscodelc.State.Stopped) { // Clear all cached statuses when clangd crashes. status.clear(); semanticHighlightingFeature.dispose();
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 @@ -44,7 +44,7 @@ // Language server push notification providing the semantic highlighting // information for a text document. -export const NotificationType = +const NotificationType = new vscodelc.NotificationType<SemanticHighlightingParams, void>( 'textDocument/semanticHighlighting'); @@ -58,6 +58,11 @@ highlighter: Highlighter; // Any disposables that should be cleaned up when clangd crashes. private subscriptions: vscode.Disposable[] = []; + private clangdClient: vscodelc.BaseLanguageClient; + + constructor(client: vscodelc.BaseLanguageClient) { + this.clangdClient = client; + } fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) { // Extend the ClientCapabilities type and add semantic highlighting // capability to the object. @@ -77,7 +82,9 @@ } initialize(capabilities: vscodelc.ServerCapabilities, - documentSelector: vscodelc.DocumentSelector|undefined) { + documentSelector: vscodelc.DocumentSelector | undefined) { + // Register the handler to handle semantic highlighting notification. + this.clangdClient.onNotification(NotificationType, this.handleNotification); // The semantic highlighting capability information is in the capabilities // object but to access the data we must first extend the ServerCapabilities // type. 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 @@ -110,7 +110,7 @@ const clangdClient = new ClangdLanguageClient('Clang Language Server', serverOptions, clientOptions); const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature(clangdClient); context.subscriptions.push( vscode.Disposable.from(semanticHighlightingFeature)); clangdClient.registerFeature(semanticHighlightingFeature); @@ -138,17 +138,15 @@ context.subscriptions.push(vscode.Disposable.from(status)); context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor( () => { status.updateStatus(); })); - context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => { - if (newState == vscodelc.State.Running) { - // clangd starts or restarts after crash. - clangdClient.onNotification( + + // The notification handler must be registered after the client is ready. + clangdClient.onReady().then( + () => {clangdClient.onNotification( 'textDocument/clangd.fileStatus', - (fileStatus) => { status.onFileUpdated(fileStatus); }); - clangdClient.onNotification( - semanticHighlighting.NotificationType, - semanticHighlightingFeature.handleNotification.bind( - semanticHighlightingFeature)); - } else if (newState == vscodelc.State.Stopped) { + (fileStatus) => { status.onFileUpdated(fileStatus); })}); + + context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => { + if (newState == vscodelc.State.Stopped) { // Clear all cached statuses when clangd crashes. status.clear(); semanticHighlightingFeature.dispose();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits