From af32b9b928d9d0267d37bab97d3deea3fa54e1aa Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sat, 16 Nov 2024 22:51:20 -0500 Subject: [PATCH 1/7] Remove dependency on ClangdContext in install.ts --- src/clangd-context.ts | 2 +- src/install.ts | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/clangd-context.ts b/src/clangd-context.ts index 51beadb..05248ea 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -62,7 +62,7 @@ export class ClangdContext implements vscode.Disposable { async activate(globalStoragePath: string, outputChannel: vscode.OutputChannel) { - const clangdPath = await install.activate(this, globalStoragePath); + const clangdPath = await install.activate(this.subscriptions, globalStoragePath); if (!clangdPath) return; diff --git a/src/install.ts b/src/install.ts index d9c3bd8..2783154 100644 --- a/src/install.ts +++ b/src/install.ts @@ -6,23 +6,22 @@ import AbortController from 'abort-controller'; import * as path from 'path'; import * as vscode from 'vscode'; -import {ClangdContext} from './clangd-context'; import * as config from './config'; // Returns the clangd path to be used, or null if clangd is not installed. export async function activate( - context: ClangdContext, globalStoragePath: string): Promise { - const ui = new UI(context, globalStoragePath); - context.subscriptions.push(vscode.commands.registerCommand( + disposables: vscode.Disposable[], globalStoragePath: string): Promise { + const ui = new UI(disposables, globalStoragePath); + disposables.push(vscode.commands.registerCommand( 'clangd.install', async () => common.installLatest(ui))); - context.subscriptions.push(vscode.commands.registerCommand( + disposables.push(vscode.commands.registerCommand( 'clangd.update', async () => common.checkUpdates(true, ui))); const status = await common.prepare(ui, config.get('checkUpdates')); return status.clangdPath; } class UI { - constructor(private context: ClangdContext, + constructor(private disposables: vscode.Disposable[], private globalStoragePath: string) {} get storagePath(): string { return this.globalStoragePath; } @@ -60,7 +59,7 @@ class UI { error(s: string) { vscode.window.showErrorMessage(s); } info(s: string) { vscode.window.showInformationMessage(s); } command(name: string, body: () => any) { - this.context.subscriptions.push( + this.disposables.push( vscode.commands.registerCommand(name, body)); } From e5e25c66ca4e1fbd0ebadee3a14890758dc06c6f Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sat, 16 Nov 2024 23:05:19 -0500 Subject: [PATCH 2/7] Guarantee that ClangdContext.client is null The code is reorganized slightly so that in the cases where the client would be null, we don't create a ClangdContext object in the first place --- src/clangd-context.ts | 24 ++++++++++++++++-------- src/extension.ts | 22 +++++++++++++--------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/clangd-context.ts b/src/clangd-context.ts index 05248ea..7b62da2 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -56,19 +56,27 @@ class EnableEditsNearCursorFeature implements vscodelc.StaticFeature { dispose() {} } +export async function createContext(globalStoragePath: string, + outputChannel: vscode.OutputChannel): Promise { + const subscriptions: vscode.Disposable[] = []; + const clangdPath = await install.activate(subscriptions, globalStoragePath); + if (!clangdPath) + return null; + + const clangdArguments = await config.get('arguments'); + + return new ClangdContext(clangdPath, clangdArguments, outputChannel); +} + export class ClangdContext implements vscode.Disposable { subscriptions: vscode.Disposable[] = []; - client!: ClangdLanguageClient; - - async activate(globalStoragePath: string, - outputChannel: vscode.OutputChannel) { - const clangdPath = await install.activate(this.subscriptions, globalStoragePath); - if (!clangdPath) - return; + client: ClangdLanguageClient; + constructor(clangdPath: string, clangdArguments: string[], + outputChannel: vscode.OutputChannel) { const clangd: vscodelc.Executable = { command: clangdPath, - args: await config.get('arguments'), + args: clangdArguments, options: {cwd: vscode.workspace.rootPath || process.cwd()} }; const traceFile = config.get('trace'); diff --git a/src/extension.ts b/src/extension.ts index fb9370a..cf279e9 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import {ClangdExtension} from '../api/vscode-clangd'; import {ClangdExtensionImpl} from './api'; -import {ClangdContext} from './clangd-context'; +import {ClangdContext, createContext} from './clangd-context'; let apiInstance: ClangdExtensionImpl|undefined; @@ -16,8 +16,7 @@ export async function activate(context: vscode.ExtensionContext): const outputChannel = vscode.window.createOutputChannel('clangd'); context.subscriptions.push(outputChannel); - const clangdContext = new ClangdContext; - context.subscriptions.push(clangdContext); + let clangdContext: ClangdContext | null = null; // An empty place holder for the activate command, otherwise we'll get an // "command is not registered" error. @@ -31,20 +30,25 @@ export async function activate(context: vscode.ExtensionContext): // stop/start cycle in this situation is pointless, and doesn't work // anyways because the client can't be stop()-ped when it's still in the // Starting state). - if (clangdContext.clientIsStarting()) { + if (clangdContext && clangdContext.clientIsStarting()) { return; } - await clangdContext.dispose(); - await clangdContext.activate(context.globalStoragePath, outputChannel); + if (clangdContext) + await clangdContext.dispose(); + clangdContext = await createContext(context.globalStoragePath, outputChannel); + if (clangdContext) + context.subscriptions.push(clangdContext); if (apiInstance) { - apiInstance.client = clangdContext.client; + apiInstance.client = clangdContext!.client; } })); let shouldCheck = false; if (vscode.workspace.getConfiguration('clangd').get('enable')) { - await clangdContext.activate(context.globalStoragePath, outputChannel); + clangdContext = await createContext(context.globalStoragePath, outputChannel); + if (clangdContext) + context.subscriptions.push(clangdContext); shouldCheck = vscode.workspace.getConfiguration('clangd').get( 'detectExtensionConflicts') ?? @@ -83,6 +87,6 @@ export async function activate(context: vscode.ExtensionContext): }, 5000); } - apiInstance = new ClangdExtensionImpl(clangdContext.client); + apiInstance = new ClangdExtensionImpl(clangdContext!.client); return apiInstance; } From 86150203fa8a14962a3d21551b7903aff9ff0880 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sat, 16 Nov 2024 23:10:43 -0500 Subject: [PATCH 3/7] Update the API to reflect that the language client could be null --- api/README.md | 5 +++++ api/vscode-clangd.d.ts | 2 +- src/api.ts | 2 +- src/extension.ts | 4 ++-- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/api/README.md b/api/README.md index f3e95d2..dd2c6fc 100644 --- a/api/README.md +++ b/api/README.md @@ -17,6 +17,11 @@ const provideHover = async (document: vscode.TextDocument, position: vscode.Posi if (clangdExtension) { const api = (await clangdExtension.activate()).getApi(CLANGD_API_VERSION); + + // Extension may be disabled or have failed to initialize + if (!api.languageClient) { + return undefined; + } const textDocument = api.languageClient.code2ProtocolConverter.asTextDocumentIdentifier(document); const range = api.languageClient.code2ProtocolConverter.asRange(new vscode.Range(position, position)); diff --git a/api/vscode-clangd.d.ts b/api/vscode-clangd.d.ts index 1bbbda4..18aa2e5 100644 --- a/api/vscode-clangd.d.ts +++ b/api/vscode-clangd.d.ts @@ -8,7 +8,7 @@ export interface ClangdApiV1 { // https://microsoft.github.io/language-server-protocol/specifications/specification-current // clangd custom requests: // https://clangd.llvm.org/extensions - languageClient: BaseLanguageClient + languageClient: BaseLanguageClient | undefined } export interface ClangdExtension { diff --git a/src/api.ts b/src/api.ts index fa9b443..a54d371 100644 --- a/src/api.ts +++ b/src/api.ts @@ -3,7 +3,7 @@ import {BaseLanguageClient} from 'vscode-languageclient'; import {ClangdApiV1, ClangdExtension} from '../api/vscode-clangd'; export class ClangdExtensionImpl implements ClangdExtension { - constructor(public client: BaseLanguageClient) {} + constructor(public client: BaseLanguageClient | undefined) {} public getApi(version: 1): ClangdApiV1; public getApi(version: number): unknown { diff --git a/src/extension.ts b/src/extension.ts index cf279e9..df16515 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -39,7 +39,7 @@ export async function activate(context: vscode.ExtensionContext): if (clangdContext) context.subscriptions.push(clangdContext); if (apiInstance) { - apiInstance.client = clangdContext!.client; + apiInstance.client = clangdContext?.client; } })); @@ -87,6 +87,6 @@ export async function activate(context: vscode.ExtensionContext): }, 5000); } - apiInstance = new ClangdExtensionImpl(clangdContext!.client); + apiInstance = new ClangdExtensionImpl(clangdContext?.client); return apiInstance; } From f7e7bfa34244ad2b634f3550f628aa37d27285eb Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sat, 16 Nov 2024 23:23:30 -0500 Subject: [PATCH 4/7] clang-format --- api/vscode-clangd.d.ts | 2 +- src/api.ts | 2 +- src/clangd-context.ts | 5 +++-- src/extension.ts | 5 +++-- src/install.ts | 8 ++++---- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/api/vscode-clangd.d.ts b/api/vscode-clangd.d.ts index 18aa2e5..6e9b00d 100644 --- a/api/vscode-clangd.d.ts +++ b/api/vscode-clangd.d.ts @@ -8,7 +8,7 @@ export interface ClangdApiV1 { // https://microsoft.github.io/language-server-protocol/specifications/specification-current // clangd custom requests: // https://clangd.llvm.org/extensions - languageClient: BaseLanguageClient | undefined + languageClient: BaseLanguageClient|undefined } export interface ClangdExtension { diff --git a/src/api.ts b/src/api.ts index a54d371..114e94c 100644 --- a/src/api.ts +++ b/src/api.ts @@ -3,7 +3,7 @@ import {BaseLanguageClient} from 'vscode-languageclient'; import {ClangdApiV1, ClangdExtension} from '../api/vscode-clangd'; export class ClangdExtensionImpl implements ClangdExtension { - constructor(public client: BaseLanguageClient | undefined) {} + constructor(public client: BaseLanguageClient|undefined) {} public getApi(version: 1): ClangdApiV1; public getApi(version: number): unknown { diff --git a/src/clangd-context.ts b/src/clangd-context.ts index 7b62da2..ecc5fa0 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -57,7 +57,8 @@ class EnableEditsNearCursorFeature implements vscodelc.StaticFeature { } export async function createContext(globalStoragePath: string, - outputChannel: vscode.OutputChannel): Promise { + outputChannel: vscode.OutputChannel): + Promise { const subscriptions: vscode.Disposable[] = []; const clangdPath = await install.activate(subscriptions, globalStoragePath); if (!clangdPath) @@ -73,7 +74,7 @@ export class ClangdContext implements vscode.Disposable { client: ClangdLanguageClient; constructor(clangdPath: string, clangdArguments: string[], - outputChannel: vscode.OutputChannel) { + outputChannel: vscode.OutputChannel) { const clangd: vscodelc.Executable = { command: clangdPath, args: clangdArguments, diff --git a/src/extension.ts b/src/extension.ts index df16515..249bebf 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -16,7 +16,7 @@ export async function activate(context: vscode.ExtensionContext): const outputChannel = vscode.window.createOutputChannel('clangd'); context.subscriptions.push(outputChannel); - let clangdContext: ClangdContext | null = null; + let clangdContext: ClangdContext|null = null; // An empty place holder for the activate command, otherwise we'll get an // "command is not registered" error. @@ -35,7 +35,8 @@ export async function activate(context: vscode.ExtensionContext): } if (clangdContext) await clangdContext.dispose(); - clangdContext = await createContext(context.globalStoragePath, outputChannel); + clangdContext = + await createContext(context.globalStoragePath, outputChannel); if (clangdContext) context.subscriptions.push(clangdContext); if (apiInstance) { diff --git a/src/install.ts b/src/install.ts index 2783154..a896a0c 100644 --- a/src/install.ts +++ b/src/install.ts @@ -9,8 +9,9 @@ import * as vscode from 'vscode'; import * as config from './config'; // Returns the clangd path to be used, or null if clangd is not installed. -export async function activate( - disposables: vscode.Disposable[], globalStoragePath: string): Promise { +export async function activate(disposables: vscode.Disposable[], + globalStoragePath: string): + Promise { const ui = new UI(disposables, globalStoragePath); disposables.push(vscode.commands.registerCommand( 'clangd.install', async () => common.installLatest(ui))); @@ -59,8 +60,7 @@ class UI { error(s: string) { vscode.window.showErrorMessage(s); } info(s: string) { vscode.window.showInformationMessage(s); } command(name: string, body: () => any) { - this.disposables.push( - vscode.commands.registerCommand(name, body)); + this.disposables.push(vscode.commands.registerCommand(name, body)); } async shouldReuse(release: string): Promise { From c7c91e644eec4cd46b62770336fac2a268426675 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sat, 16 Nov 2024 23:25:08 -0500 Subject: [PATCH 5/7] more clang-format --- src/clangd-context.ts | 2 +- src/extension.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/clangd-context.ts b/src/clangd-context.ts index ecc5fa0..71f0f8a 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -58,7 +58,7 @@ class EnableEditsNearCursorFeature implements vscodelc.StaticFeature { export async function createContext(globalStoragePath: string, outputChannel: vscode.OutputChannel): - Promise { + Promise { const subscriptions: vscode.Disposable[] = []; const clangdPath = await install.activate(subscriptions, globalStoragePath); if (!clangdPath) diff --git a/src/extension.ts b/src/extension.ts index 249bebf..0848804 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -47,7 +47,8 @@ export async function activate(context: vscode.ExtensionContext): let shouldCheck = false; if (vscode.workspace.getConfiguration('clangd').get('enable')) { - clangdContext = await createContext(context.globalStoragePath, outputChannel); + clangdContext = + await createContext(context.globalStoragePath, outputChannel); if (clangdContext) context.subscriptions.push(clangdContext); From 9ece44c8c8644c17c085a990d39e539deed81a59 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 17 Nov 2024 17:15:08 -0500 Subject: [PATCH 6/7] address review comments --- src/clangd-context.ts | 33 +++++++++++++++++---------------- src/extension.ts | 8 ++++---- src/install.ts | 2 +- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/clangd-context.ts b/src/clangd-context.ts index 71f0f8a..eeed34d 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -56,25 +56,25 @@ class EnableEditsNearCursorFeature implements vscodelc.StaticFeature { dispose() {} } -export async function createContext(globalStoragePath: string, - outputChannel: vscode.OutputChannel): - Promise { - const subscriptions: vscode.Disposable[] = []; - const clangdPath = await install.activate(subscriptions, globalStoragePath); - if (!clangdPath) - return null; - - const clangdArguments = await config.get('arguments'); - - return new ClangdContext(clangdPath, clangdArguments, outputChannel); -} - export class ClangdContext implements vscode.Disposable { subscriptions: vscode.Disposable[] = []; client: ClangdLanguageClient; - constructor(clangdPath: string, clangdArguments: string[], - outputChannel: vscode.OutputChannel) { + static async create(globalStoragePath: string, + outputChannel: vscode.OutputChannel): + Promise { + const subscriptions: vscode.Disposable[] = []; + const clangdPath = await install.activate(subscriptions, globalStoragePath); + if (!clangdPath) + return null; + + const clangdArguments = await config.get('arguments'); + + return new ClangdContext(clangdPath, clangdArguments, outputChannel); + } + + private constructor(clangdPath: string, clangdArguments: string[], + outputChannel: vscode.OutputChannel) { const clangd: vscodelc.Executable = { command: clangdPath, args: clangdArguments, @@ -120,7 +120,8 @@ export class ClangdContext implements vscode.Disposable { let list = await next(document, position, context, token); if (!config.get('serverCompletionRanking')) return list; - let items = (Array.isArray(list) ? list : list!.items).map(item => { + let items = (!list ? [] : Array.isArray(list) ? list : list.items); + items = items.map(item => { // Gets the prefix used by VSCode when doing fuzzymatch. let prefix = document.getText( new vscode.Range((item.range as vscode.Range).start, position)) diff --git a/src/extension.ts b/src/extension.ts index 0848804..8d74615 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import {ClangdExtension} from '../api/vscode-clangd'; import {ClangdExtensionImpl} from './api'; -import {ClangdContext, createContext} from './clangd-context'; +import {ClangdContext} from './clangd-context'; let apiInstance: ClangdExtensionImpl|undefined; @@ -34,9 +34,9 @@ export async function activate(context: vscode.ExtensionContext): return; } if (clangdContext) - await clangdContext.dispose(); + clangdContext.dispose(); clangdContext = - await createContext(context.globalStoragePath, outputChannel); + await ClangdContext.create(context.globalStoragePath, outputChannel); if (clangdContext) context.subscriptions.push(clangdContext); if (apiInstance) { @@ -48,7 +48,7 @@ export async function activate(context: vscode.ExtensionContext): if (vscode.workspace.getConfiguration('clangd').get('enable')) { clangdContext = - await createContext(context.globalStoragePath, outputChannel); + await ClangdContext.create(context.globalStoragePath, outputChannel); if (clangdContext) context.subscriptions.push(clangdContext); diff --git a/src/install.ts b/src/install.ts index a896a0c..b333c40 100644 --- a/src/install.ts +++ b/src/install.ts @@ -120,7 +120,7 @@ class UI { } get clangdPath(): string { - let p = config.get('path')!; + let p = config.get('path'); // Backwards compatibility: if it's a relative path with a slash, interpret // relative to project root. if (!path.isAbsolute(p) && p.includes(path.sep) && From f506cf23c89e6ed4683d7c2602c91971b9341cef Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 17 Nov 2024 17:16:51 -0500 Subject: [PATCH 7/7] clang-format --- src/clangd-context.ts | 2 +- src/extension.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clangd-context.ts b/src/clangd-context.ts index eeed34d..db2412b 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -66,7 +66,7 @@ export class ClangdContext implements vscode.Disposable { const subscriptions: vscode.Disposable[] = []; const clangdPath = await install.activate(subscriptions, globalStoragePath); if (!clangdPath) - return null; + return null; const clangdArguments = await config.get('arguments'); diff --git a/src/extension.ts b/src/extension.ts index 8d74615..2f9a79c 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -35,8 +35,8 @@ export async function activate(context: vscode.ExtensionContext): } if (clangdContext) clangdContext.dispose(); - clangdContext = - await ClangdContext.create(context.globalStoragePath, outputChannel); + clangdContext = await ClangdContext.create(context.globalStoragePath, + outputChannel); if (clangdContext) context.subscriptions.push(clangdContext); if (apiInstance) {