From 2b00cc43313d6b0cc5b717d751b5bea1321edb33 Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Fri, 12 Jan 2024 17:05:51 -0800 Subject: [PATCH 1/2] Fix up debugger configuration resolvers Ok so we removed the notion of IDs (which are integers) being strings because that broke serialization with recent updates. But on inspection we realized the whole point was to be able to say "current" and have the attach configuration attach to the currently running Extension Terminal. Except that this was an unsupported, half-baked scenario (sure, it attached, but it couldn't actually be debugged properly). So now that throws warnings in both the client and server. The defaults for `processId` and `runspaceId` where changed to `null` (again, now they're just integers) which means to query for them. The supplied but half-baked configuration was removed. --- package.json | 31 ++++------- src/features/DebugSession.ts | 100 ++++++++++++++++++++--------------- src/session.ts | 7 +++ 3 files changed, 74 insertions(+), 64 deletions(-) diff --git a/package.json b/package.json index 9b9e2f9e4e..8404f63f73 100644 --- a/package.json +++ b/package.json @@ -486,18 +486,7 @@ "body": { "name": "PowerShell Attach to Host Process", "type": "PowerShell", - "request": "attach", - "runspaceId": 1 - } - }, - { - "label": "PowerShell: Attach Interactive Session Runspace", - "description": "Open runspace picker to select runspace to attach debugger to", - "body": { - "name": "PowerShell Attach Interactive Session Runspace", - "type": "PowerShell", - "request": "attach", - "processId": "current" + "request": "attach" } }, { @@ -588,29 +577,27 @@ "properties": { "computerName": { "type": "string", - "description": "Optional: The computer name to which a remote session will be established. Works only on PowerShell 4 and above." + "description": "Optional: The computer name to which a remote session will be established.", + "default": null }, "processId": { - "type": "string", - "description": "The process id of the PowerShell host process to attach to. Works only on PowerShell 5 and above.", + "type": "number", + "description": "Optional: The ID of the PowerShell host process that should be attached. Will prompt if unspecified.", "default": null }, "runspaceId": { - "type": [ - "string", - "number" - ], - "description": "Optional: The ID of the runspace to debug in the attached process. Defaults to 1. Works only on PowerShell 5 and above.", + "type": "number", + "description": "Optional: The ID of the runspace to debug in the attached process. Will prompt if unspecified.", "default": null }, "runspaceName": { "type": "string", - "description": "Optional: The Name of the runspace to debug in the attached process. Works only on PowerShell 5 and above.", + "description": "Optional: The name of the runspace to debug in the attached process.", "default": null }, "customPipeName": { "type": "string", - "description": "The custom pipe name of the PowerShell host process to attach to. Works only on PowerShell 6.2 and above.", + "description": "The custom pipe name of the PowerShell host process to attach to.", "default": null } } diff --git a/src/features/DebugSession.ts b/src/features/DebugSession.ts index d0412849b9..701c8a8862 100644 --- a/src/features/DebugSession.ts +++ b/src/features/DebugSession.ts @@ -87,7 +87,6 @@ export const DebugConfigurations: Record = { name: "PowerShell: Attach to PowerShell Host Process", type: "PowerShell", request: "attach", - runspaceId: 1, }, [DebugConfig.RunPester]: { name: "PowerShell: Run Pester Tests", @@ -95,7 +94,7 @@ export const DebugConfigurations: Record = { request: "launch", script: "Invoke-Pester", createTemporaryIntegratedConsole: true, - attachDotnetDebugger: true + attachDotnetDebugger: true, }, [DebugConfig.ModuleInteractiveSession]: { name: "PowerShell: Module Interactive Session", @@ -109,7 +108,7 @@ export const DebugConfigurations: Record = { request: "launch", script: "Enter command to import your binary module, for example: \"Import-Module -Force ${workspaceFolder}/path/to/module.psd1|dll\"", createTemporaryIntegratedConsole: true, - attachDotnetDebugger: true + attachDotnetDebugger: true, }, [DebugConfig.BinaryModulePester]: { name: "PowerShell: Binary Module Pester Tests", @@ -117,7 +116,7 @@ export const DebugConfigurations: Record = { request: "launch", script: "Invoke-Pester", createTemporaryIntegratedConsole: true, - attachDotnetDebugger: true + attachDotnetDebugger: true, } }; @@ -131,7 +130,7 @@ export class DebugSessionFeature extends LanguageClientConsumer constructor(context: ExtensionContext, private sessionManager: SessionManager, private logger: ILogger) { super(); - // This "activates" the debug adapter for use with You can only do this once. + // This "activates" the debug adapter. You can only do this once. [ DebugConfigurationProviderTriggerKind.Initial, DebugConfigurationProviderTriggerKind.Dynamic @@ -276,11 +275,15 @@ export class DebugSessionFeature extends LanguageClientConsumer return resolvedConfig; } - // This is our factory entrypoint hook to when a debug session starts, and where we will lazy initialize everything needed to do the debugging such as a temporary console if required + // This is our factory entrypoint hook to when a debug session starts, and + // where we will lazy initialize everything needed to do the debugging such + // as a temporary console if required. + // + // NOTE: A Promise meets the shape of a ProviderResult, which allows us to + // make this method async. public async createDebugAdapterDescriptor( session: DebugSession, _executable: DebugAdapterExecutable | undefined): Promise { - // NOTE: A Promise meets the shape of a ProviderResult, which allows us to make this method async. await this.sessionManager.start(); @@ -426,6 +429,7 @@ export class DebugSessionFeature extends LanguageClientConsumer this.logger.writeVerbose(`Dotnet attach debug configuration: ${JSON.stringify(dotnetAttachConfig, undefined, 2)}`); this.logger.writeVerbose(`Attached dotnet debugger to process: ${pid}`); } + return this.tempSessionDetails; } @@ -452,7 +456,7 @@ export class DebugSessionFeature extends LanguageClientConsumer }; } - /** Fetches all available vscode launch configurations. This is abstracted out for easier testing */ + /** Fetches all available vscode launch configurations. This is abstracted out for easier testing. */ private getLaunchConfigurations(): DebugConfiguration[] { return workspace.getConfiguration("launch").get("configurations") ?? []; } @@ -480,6 +484,23 @@ export class DebugSessionFeature extends LanguageClientConsumer } } + // If we were given a stringified int from the user, or from the picker + // command, we need to parse it here. + if (typeof config.processId === "string" && config.processId != "current") { + config.processId = parseInt(config.processId); + } + + // NOTE: We don't support attaching to the Extension Terminal, even + // though in the past it looked like we did. The implementation was + // half-baked and left things unusable. + if (config.processId === "current" || config.processId === await this.sessionManager.getLanguageServerPid()) { + // TODO: When (if ever) it's supported, we need to convert 0 and the + // old notion of "current" to the actual process ID, like this: + // config.processId = await this.sessionManager.getLanguageServerPid(); + void this.logger.writeAndShowError("Attaching to the PowerShell Extension terminal is not supported. Please use the 'PowerShell: Interactive Session' debug configuration instead."); + return PREVENT_DEBUG_START_AND_OPEN_DEBUGCONFIG; + } + if (!config.runspaceId && !config.runspaceName) { config.runspaceId = await commands.executeCommand("PowerShell.PickRunspace", config.processId); // No runspace selected. Cancel attach. @@ -528,12 +549,13 @@ export class SpecifyScriptArgsFeature implements Disposable { if (text !== undefined) { await this.context.workspaceState.update(powerShellDbgScriptArgsKey, text); } + return text; } } interface IProcessItem extends QuickPickItem { - pid: string; // payload for the QuickPick UI + processId: number; // payload for the QuickPick UI } // eslint-disable-next-line @typescript-eslint/no-empty-interface @@ -542,7 +564,7 @@ interface IGetPSHostProcessesArguments { interface IPSHostProcessInfo { processName: string; - processId: string; + processId: number; appDomainName: string; mainWindowTitle: string; } @@ -551,7 +573,6 @@ export const GetPSHostProcessesRequestType = new RequestType("powerShell/getPSHostProcesses"); export class PickPSHostProcessFeature extends LanguageClientConsumer { - private command: Disposable; private waitingForClientToken?: CancellationTokenSource; private getLanguageClientResolve?: (value: LanguageClient) => void; @@ -559,11 +580,9 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer { constructor(private logger: ILogger) { super(); - this.command = - commands.registerCommand("PowerShell.PickPSHostProcess", () => { - return this.getLanguageClient() - .then((_) => this.pickPSHostProcess(), (_) => undefined); - }); + this.command = commands.registerCommand("PowerShell.PickPSHostProcess", async () => { + return this.pickPSHostProcess(); + }); } public override setLanguageClient(languageClient: LanguageClient): void { @@ -617,25 +636,24 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer { } } - private async pickPSHostProcess(): Promise { + private async pickPSHostProcess(): Promise { + // We need the language client in order to send the request. + await this.getLanguageClient(); + // Start with the current PowerShell process in the list. - const items: IProcessItem[] = [{ - label: "Current", - description: "The current PowerShell Extension process.", - pid: "current", - }]; + const items: IProcessItem[] = []; const response = await this.languageClient?.sendRequest(GetPSHostProcessesRequestType, {}); for (const process of response ?? []) { let windowTitle = ""; if (process.mainWindowTitle) { - windowTitle = `, Title: ${process.mainWindowTitle}`; + windowTitle = `, ${process.mainWindowTitle}`; } items.push({ label: process.processName, description: `PID: ${process.processId.toString()}${windowTitle}`, - pid: process.processId, + processId: process.processId, }); } @@ -648,9 +666,10 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer { matchOnDescription: true, matchOnDetail: true, }; + const item = await window.showQuickPick(items, options); - return item ? item.pid : undefined; + return item?.processId ?? undefined; } private clearWaitingToken(): void { @@ -660,7 +679,7 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer { } interface IRunspaceItem extends QuickPickItem { - id: string; // payload for the QuickPick UI + id: number; // payload for the QuickPick UI } // eslint-disable-next-line @typescript-eslint/no-empty-interface @@ -677,18 +696,16 @@ export const GetRunspaceRequestType = new RequestType("powerShell/getRunspace"); export class PickRunspaceFeature extends LanguageClientConsumer { - private command: Disposable; private waitingForClientToken?: CancellationTokenSource; private getLanguageClientResolve?: (value: LanguageClient) => void; constructor(private logger: ILogger) { super(); - this.command = - commands.registerCommand("PowerShell.PickRunspace", (processId) => { - return this.getLanguageClient() - .then((_) => this.pickRunspace(processId), (_) => undefined); - }, this); + + this.command = commands.registerCommand("PowerShell.PickRunspace", + async (processId) => { return this.pickRunspace(processId); }, + this); } public override setLanguageClient(languageClient: LanguageClient): void { @@ -734,7 +751,8 @@ export class PickRunspaceFeature extends LanguageClientConsumer { this.clearWaitingToken(); reject(); - void this.logger.writeAndShowError("Attach to PowerShell host process: PowerShell session took too long to start."); + void this.logger.writeAndShowError( + "Attach to PowerShell host process: PowerShell session took too long to start."); } }, 60000); }, @@ -742,20 +760,17 @@ export class PickRunspaceFeature extends LanguageClientConsumer { } } - private async pickRunspace(processId: string): Promise { + private async pickRunspace(processId: number): Promise { + // We need the language client in order to send the request. + await this.getLanguageClient(); + const response = await this.languageClient?.sendRequest(GetRunspaceRequestType, { processId }); const items: IRunspaceItem[] = []; for (const runspace of response ?? []) { - // Skip default runspace - if ((runspace.id === 1 || runspace.name === "PSAttachRunspace") - && processId === "current") { - continue; - } - items.push({ label: runspace.name, description: `ID: ${runspace.id} - ${runspace.availability}`, - id: runspace.id.toString(), + id: runspace.id, }); } @@ -764,9 +779,10 @@ export class PickRunspaceFeature extends LanguageClientConsumer { matchOnDescription: true, matchOnDetail: true, }; + const item = await window.showQuickPick(items, options); - return item ? item.id : undefined; + return item?.id ?? undefined; } private clearWaitingToken(): void { diff --git a/src/session.ts b/src/session.ts index 856a63ba97..df896cfbc5 100644 --- a/src/session.ts +++ b/src/session.ts @@ -303,6 +303,13 @@ export class SessionManager implements Middleware { return this.sessionDetails; } + public async getLanguageServerPid(): Promise { + if (this.languageServerProcess === undefined) { + void this.logger.writeAndShowError("PowerShell Extension Terminal unavailable!"); + } + return this.languageServerProcess?.getPid(); + } + public getPowerShellVersionDetails(): IPowerShellVersionDetails | undefined { return this.versionDetails; } From d429bebc3315b73fa3d6be5f108696544f78e540 Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Mon, 22 Jan 2024 19:45:09 -0800 Subject: [PATCH 2/2] Refactor `LanguageClientConsumer` to wait properly This makes a lot more sense and fixes a long-standing bug. Also cleaned up a lot while at it. 1. `SessionManager` is created and the features are injected into the session manager with the intention of them being able to "register" their language client event handlers in the period between when the client is created and when it is started. 2. For each feature, if it gets to a point where it needs the language client, we want them to wait on a promise for the "started" client to be provided from the session manager before continuing. 3. Once it is started, the session manager passes the client to `LanguageClientConsumer` which resolves the promise for the features so they can continue. --- src/features/CodeActions.ts | 8 +- src/features/Console.ts | 12 +- src/features/DebugSession.ts | 308 +++++++++-------------------- src/features/ExpandAlias.ts | 18 +- src/features/ExtensionCommands.ts | 52 +++-- src/features/ExternalApi.ts | 4 +- src/features/GetCommands.ts | 25 +-- src/features/HelpCompletion.ts | 24 +-- src/features/ISECompatibility.ts | 22 +-- src/features/NewFileOrProject.ts | 115 ++++------- src/features/RemoteFiles.ts | 9 +- src/features/ShowHelp.ts | 10 +- src/languageClientConsumer.ts | 64 ++++-- src/main.ts | 10 +- src/session.ts | 5 +- test/features/DebugSession.test.ts | 56 ++++-- 16 files changed, 318 insertions(+), 424 deletions(-) diff --git a/src/features/CodeActions.ts b/src/features/CodeActions.ts index 4be6ac9ed6..1673b3e39f 100644 --- a/src/features/CodeActions.ts +++ b/src/features/CodeActions.ts @@ -5,7 +5,7 @@ import vscode = require("vscode"); import { ILogger } from "../logging"; export class CodeActionsFeature implements vscode.Disposable { - private showDocumentationCommand: vscode.Disposable; + private command: vscode.Disposable; constructor(private log: ILogger) { // NOTE: While not exposed to the user via package.json, this is @@ -13,17 +13,17 @@ export class CodeActionsFeature implements vscode.Disposable { // // TODO: In the far future with LSP 3.19 the server can just set a URL // and this can go away. See https://github.com/microsoft/language-server-protocol/issues/1548 - this.showDocumentationCommand = + this.command = vscode.commands.registerCommand("PowerShell.ShowCodeActionDocumentation", async (ruleName: string) => { await this.showRuleDocumentation(ruleName); }); } public dispose(): void { - this.showDocumentationCommand.dispose(); + this.command.dispose(); } - public async showRuleDocumentation(ruleId: string): Promise { + private async showRuleDocumentation(ruleId: string): Promise { const pssaDocBaseURL = "https://docs.microsoft.com/powershell/utility-modules/psscriptanalyzer/rules/"; if (!ruleId) { diff --git a/src/features/Console.ts b/src/features/Console.ts index 22fcf83913..01cb5c6915 100644 --- a/src/features/Console.ts +++ b/src/features/Console.ts @@ -200,8 +200,8 @@ export class ConsoleFeature extends LanguageClientConsumer { } else { selectionRange = editor.document.lineAt(editor.selection.start.line).range; } - - await this.languageClient?.sendRequest(EvaluateRequestType, { + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendRequest(EvaluateRequestType, { expression: editor.document.getText(selectionRange), }); @@ -217,19 +217,19 @@ export class ConsoleFeature extends LanguageClientConsumer { for (const command of this.commands) { command.dispose(); } + for (const handler of this.handlers) { handler.dispose(); } } - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; + public override onLanguageClientSet(languageClient: LanguageClient): void { this.handlers = [ - this.languageClient.onRequest( + languageClient.onRequest( ShowChoicePromptRequestType, (promptDetails) => showChoicePrompt(promptDetails)), - this.languageClient.onRequest( + languageClient.onRequest( ShowInputPromptRequestType, (promptDetails) => showInputPrompt(promptDetails)), ]; diff --git a/src/features/DebugSession.ts b/src/features/DebugSession.ts index 701c8a8862..8aeb26620a 100644 --- a/src/features/DebugSession.ts +++ b/src/features/DebugSession.ts @@ -126,27 +126,56 @@ export class DebugSessionFeature extends LanguageClientConsumer private sessionCount = 1; private tempDebugProcess: PowerShellProcess | undefined; private tempSessionDetails: IEditorServicesSessionDetails | undefined; + private commands: Disposable[] = []; private handlers: Disposable[] = []; constructor(context: ExtensionContext, private sessionManager: SessionManager, private logger: ILogger) { super(); - // This "activates" the debug adapter. You can only do this once. - [ - DebugConfigurationProviderTriggerKind.Initial, - DebugConfigurationProviderTriggerKind.Dynamic - ].forEach(triggerKind => { - context.subscriptions.push(debug.registerDebugConfigurationProvider("PowerShell", this, triggerKind)); - }); - context.subscriptions.push(debug.registerDebugAdapterDescriptorFactory("PowerShell", this)); + + this.activateDebugAdaptor(context); + + // NOTE: While process and runspace IDs are numbers, command + // substitutions in VS Code debug configurations are required to return + // strings. Hence to the `toString()` on these. + this.commands = [ + commands.registerCommand("PowerShell.PickPSHostProcess", async () => { + const processId = await this.pickPSHostProcess(); + return processId?.toString(); + }), + + commands.registerCommand("PowerShell.PickRunspace", async (processId) => { + const runspace = await this.pickRunspace(processId); + return runspace?.toString(); + }, this), + ]; } public dispose(): void { + for (const command of this.commands) { + command.dispose(); + } + for (const handler of this.handlers) { handler.dispose(); } } - public override setLanguageClient(languageClient: LanguageClient): void { + // This "activates" the debug adapter. You can only do this once. + public activateDebugAdaptor(context: ExtensionContext): void { + const triggers = [ + DebugConfigurationProviderTriggerKind.Initial, + DebugConfigurationProviderTriggerKind.Dynamic + ]; + + for (const triggerKind of triggers) { + context.subscriptions.push( + debug.registerDebugConfigurationProvider("PowerShell", this, triggerKind)); + } + + context.subscriptions.push(debug.registerDebugAdapterDescriptorFactory("PowerShell", this)); + } + + public override onLanguageClientSet(languageClient: LanguageClient): void { this.handlers = [ languageClient.onNotification( StartDebuggerNotificationType, @@ -477,7 +506,7 @@ export class DebugSessionFeature extends LanguageClientConsumer // If nothing is set, prompt for the processId. if (!config.customPipeName && !config.processId) { - config.processId = await commands.executeCommand("PowerShell.PickPSHostProcess"); + config.processId = await this.pickPSHostProcess(); // No process selected. Cancel attach. if (!config.processId) { return PREVENT_DEBUG_START; @@ -502,7 +531,7 @@ export class DebugSessionFeature extends LanguageClientConsumer } if (!config.runspaceId && !config.runspaceName) { - config.runspaceId = await commands.executeCommand("PowerShell.PickRunspace", config.processId); + config.runspaceId = await this.pickRunspace(config.processId); // No runspace selected. Cancel attach. if (!config.runspaceId) { return PREVENT_DEBUG_START; @@ -511,10 +540,64 @@ export class DebugSessionFeature extends LanguageClientConsumer return config; } + + private async pickPSHostProcess(): Promise { + const client = await LanguageClientConsumer.getLanguageClient(); + const response = await client.sendRequest(GetPSHostProcessesRequestType, {}); + const items: IProcessItem[] = []; + for (const process of response) { + let windowTitle = ""; + if (process.mainWindowTitle) { + windowTitle = `, ${process.mainWindowTitle}`; + } + + items.push({ + label: process.processName, + description: `PID: ${process.processId.toString()}${windowTitle}`, + processId: process.processId, + }); + } + + if (items.length === 0) { + return Promise.reject("There are no PowerShell host processes to attach."); + } + + const options: QuickPickOptions = { + placeHolder: "Select a PowerShell host process to attach.", + matchOnDescription: true, + matchOnDetail: true, + }; + + const item = await window.showQuickPick(items, options); + + return item?.processId ?? undefined; + } + + private async pickRunspace(processId: number): Promise { + const client = await LanguageClientConsumer.getLanguageClient(); + const response = await client.sendRequest(GetRunspaceRequestType, { processId }); + const items: IRunspaceItem[] = []; + for (const runspace of response) { + items.push({ + label: runspace.name, + description: `ID: ${runspace.id} - ${runspace.availability}`, + id: runspace.id, + }); + } + + const options: QuickPickOptions = { + placeHolder: "Select PowerShell runspace to debug", + matchOnDescription: true, + matchOnDetail: true, + }; + + const item = await window.showQuickPick(items, options); + + return item?.id ?? undefined; + } } export class SpecifyScriptArgsFeature implements Disposable { - private command: Disposable; private context: ExtensionContext; @@ -572,111 +655,6 @@ interface IPSHostProcessInfo { export const GetPSHostProcessesRequestType = new RequestType("powerShell/getPSHostProcesses"); -export class PickPSHostProcessFeature extends LanguageClientConsumer { - private command: Disposable; - private waitingForClientToken?: CancellationTokenSource; - private getLanguageClientResolve?: (value: LanguageClient) => void; - - constructor(private logger: ILogger) { - super(); - - this.command = commands.registerCommand("PowerShell.PickPSHostProcess", async () => { - return this.pickPSHostProcess(); - }); - } - - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; - - if (this.waitingForClientToken && this.getLanguageClientResolve) { - this.getLanguageClientResolve(this.languageClient); - this.clearWaitingToken(); - } - } - - public dispose(): void { - this.command.dispose(); - } - - private getLanguageClient(): Promise { - if (this.languageClient !== undefined) { - return Promise.resolve(this.languageClient); - } else { - // If PowerShell isn't finished loading yet, show a loading message - // until the LanguageClient is passed on to us - this.waitingForClientToken = new CancellationTokenSource(); - - return new Promise( - (resolve, reject) => { - this.getLanguageClientResolve = resolve; - - void window - .showQuickPick( - ["Cancel"], - { placeHolder: "Attach to PowerShell host process: Please wait, starting PowerShell..." }, - this.waitingForClientToken?.token) - .then((response) => { - if (response === "Cancel") { - this.clearWaitingToken(); - reject(); - } - }, undefined); - - // Cancel the loading prompt after 60 seconds - setTimeout(() => { - if (this.waitingForClientToken) { - this.clearWaitingToken(); - reject(); - - void this.logger.writeAndShowError("Attach to PowerShell host process: PowerShell session took too long to start."); - } - }, 60000); - }, - ); - } - } - - private async pickPSHostProcess(): Promise { - // We need the language client in order to send the request. - await this.getLanguageClient(); - - // Start with the current PowerShell process in the list. - const items: IProcessItem[] = []; - - const response = await this.languageClient?.sendRequest(GetPSHostProcessesRequestType, {}); - for (const process of response ?? []) { - let windowTitle = ""; - if (process.mainWindowTitle) { - windowTitle = `, ${process.mainWindowTitle}`; - } - - items.push({ - label: process.processName, - description: `PID: ${process.processId.toString()}${windowTitle}`, - processId: process.processId, - }); - } - - if (items.length === 0) { - return Promise.reject("There are no PowerShell host processes to attach to."); - } - - const options: QuickPickOptions = { - placeHolder: "Select a PowerShell host process to attach to", - matchOnDescription: true, - matchOnDetail: true, - }; - - const item = await window.showQuickPick(items, options); - - return item?.processId ?? undefined; - } - - private clearWaitingToken(): void { - this.waitingForClientToken?.dispose(); - this.waitingForClientToken = undefined; - } -} interface IRunspaceItem extends QuickPickItem { id: number; // payload for the QuickPick UI @@ -694,99 +672,3 @@ interface IRunspace { export const GetRunspaceRequestType = new RequestType("powerShell/getRunspace"); - -export class PickRunspaceFeature extends LanguageClientConsumer { - private command: Disposable; - private waitingForClientToken?: CancellationTokenSource; - private getLanguageClientResolve?: (value: LanguageClient) => void; - - constructor(private logger: ILogger) { - super(); - - this.command = commands.registerCommand("PowerShell.PickRunspace", - async (processId) => { return this.pickRunspace(processId); }, - this); - } - - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; - - if (this.waitingForClientToken && this.getLanguageClientResolve) { - this.getLanguageClientResolve(this.languageClient); - this.clearWaitingToken(); - } - } - - public dispose(): void { - this.command.dispose(); - } - - private getLanguageClient(): Promise { - if (this.languageClient) { - return Promise.resolve(this.languageClient); - } else { - // If PowerShell isn't finished loading yet, show a loading message - // until the LanguageClient is passed on to us - this.waitingForClientToken = new CancellationTokenSource(); - - return new Promise( - (resolve, reject) => { - this.getLanguageClientResolve = resolve; - - void window - .showQuickPick( - ["Cancel"], - { placeHolder: "Attach to PowerShell host process: Please wait, starting PowerShell..." }, - this.waitingForClientToken?.token) - .then((response) => { - if (response === "Cancel") { - this.clearWaitingToken(); - reject(); - } - }, undefined); - - // Cancel the loading prompt after 60 seconds - setTimeout(() => { - if (this.waitingForClientToken) { - this.clearWaitingToken(); - reject(); - - void this.logger.writeAndShowError( - "Attach to PowerShell host process: PowerShell session took too long to start."); - } - }, 60000); - }, - ); - } - } - - private async pickRunspace(processId: number): Promise { - // We need the language client in order to send the request. - await this.getLanguageClient(); - - const response = await this.languageClient?.sendRequest(GetRunspaceRequestType, { processId }); - const items: IRunspaceItem[] = []; - for (const runspace of response ?? []) { - items.push({ - label: runspace.name, - description: `ID: ${runspace.id} - ${runspace.availability}`, - id: runspace.id, - }); - } - - const options: QuickPickOptions = { - placeHolder: "Select PowerShell runspace to debug", - matchOnDescription: true, - matchOnDetail: true, - }; - - const item = await window.showQuickPick(items, options); - - return item?.id ?? undefined; - } - - private clearWaitingToken(): void { - this.waitingForClientToken?.dispose(); - this.waitingForClientToken = undefined; - } -} diff --git a/src/features/ExpandAlias.ts b/src/features/ExpandAlias.ts index 32b6bb8d77..94ad8f95ab 100644 --- a/src/features/ExpandAlias.ts +++ b/src/features/ExpandAlias.ts @@ -2,9 +2,9 @@ // Licensed under the MIT License. import vscode = require("vscode"); -import Window = vscode.window; import { RequestType } from "vscode-languageclient"; import { LanguageClientConsumer } from "../languageClientConsumer"; +import type { LanguageClient } from "vscode-languageclient/node"; // eslint-disable-next-line @typescript-eslint/no-empty-interface interface IExpandAliasRequestArguments { @@ -23,7 +23,7 @@ export class ExpandAliasFeature extends LanguageClientConsumer { constructor() { super(); this.command = vscode.commands.registerCommand("PowerShell.ExpandAlias", async () => { - const editor = Window.activeTextEditor; + const editor = vscode.window.activeTextEditor; if (editor === undefined) { return; } @@ -44,15 +44,17 @@ export class ExpandAliasFeature extends LanguageClientConsumer { range = new vscode.Range(sls.line, sls.character, sle.line, sle.character); } - const result = await this.languageClient?.sendRequest(ExpandAliasRequestType, { text }); - if (result !== undefined) { - await editor.edit((editBuilder) => { - editBuilder.replace(range, result.text); - }); - } + const client = await LanguageClientConsumer.getLanguageClient(); + const result = await client.sendRequest(ExpandAliasRequestType, { text }); + await editor.edit((editBuilder) => { + editBuilder.replace(range, result.text); + }); }); } + // eslint-disable-next-line @typescript-eslint/no-empty-function + public override onLanguageClientSet(_languageClient: LanguageClient): void {} + public dispose(): void { this.command.dispose(); } diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index 3f8f77d48e..f7bd5d09f6 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -157,9 +157,7 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer { super(); this.commands = [ vscode.commands.registerCommand("PowerShell.ShowAdditionalCommands", async () => { - if (this.languageClient !== undefined) { - await this.showExtensionCommands(this.languageClient); - } + await this.showExtensionCommands(); }), vscode.commands.registerCommand("PowerShell.InvokeRegisteredEditorCommand", @@ -168,7 +166,9 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer { (x) => x.name === param.commandName); if (commandToExecute) { - await this.languageClient?.sendRequest( + + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendRequest( InvokeExtensionCommandRequestType, { name: commandToExecute.name, @@ -193,63 +193,60 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer { ]; } - public override setLanguageClient(languageClient: LanguageClient): void { + public override onLanguageClientSet(languageClient: LanguageClient): void { // Clear the current list of extension commands since they were // only relevant to the previous session this.extensionCommands = []; - - this.languageClient = languageClient; - this.handlers = [ - this.languageClient.onNotification( + languageClient.onNotification( ExtensionCommandAddedNotificationType, (command) => { this.addExtensionCommand(command); }), - this.languageClient.onRequest( + languageClient.onRequest( GetEditorContextRequestType, (_details) => this.getEditorContext()), - this.languageClient.onRequest( + languageClient.onRequest( InsertTextRequestType, (details) => this.insertText(details)), - this.languageClient.onRequest( + languageClient.onRequest( SetSelectionRequestType, (details) => this.setSelection(details)), - this.languageClient.onRequest( + languageClient.onRequest( NewFileRequestType, (_content) => this.newFile(_content)), - this.languageClient.onRequest( + languageClient.onRequest( OpenFileRequestType, (filePath) => this.openFile(filePath)), - this.languageClient.onRequest( + languageClient.onRequest( CloseFileRequestType, (filePath) => this.closeFile(filePath)), - this.languageClient.onRequest( + languageClient.onRequest( SaveFileRequestType, (saveFileDetails) => this.saveFile(saveFileDetails)), - this.languageClient.onRequest( + languageClient.onRequest( ShowInformationMessageRequestType, (message) => this.showInformationMessage(message)), - this.languageClient.onRequest( + languageClient.onRequest( ShowErrorMessageRequestType, (message) => this.showErrorMessage(message)), - this.languageClient.onRequest( + languageClient.onRequest( ShowWarningMessageRequestType, (message) => this.showWarningMessage(message)), - this.languageClient.onRequest( + languageClient.onRequest( SetStatusBarMessageRequestType, (messageDetails) => this.setStatusBarMessage(messageDetails)), - this.languageClient.onNotification( + languageClient.onNotification( ClearTerminalNotificationType, () => { // We check to see if they have TrueClear on. If not, no-op because the @@ -284,7 +281,7 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer { a.name.localeCompare(b.name)); } - private async showExtensionCommands(client: LanguageClient): Promise { + private async showExtensionCommands(): Promise { // If no extension commands are available, show a message if (this.extensionCommands.length === 0) { void this.logger.writeAndShowInformation("No extension commands have been loaded into the current session."); @@ -303,15 +300,14 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer { const selectedCommand = await vscode.window.showQuickPick( quickPickItems, { placeHolder: "Select a command..." }); - return this.onCommandSelected(selectedCommand, client); - } - private async onCommandSelected( - chosenItem: IExtensionCommandQuickPickItem | undefined, - client: LanguageClient | undefined): Promise { + return this.onCommandSelected(selectedCommand); + } + private async onCommandSelected(chosenItem?: IExtensionCommandQuickPickItem): Promise { if (chosenItem !== undefined) { - await client?.sendRequest( + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendRequest( InvokeExtensionCommandRequestType, { name: chosenItem.command.name, diff --git a/src/features/ExternalApi.ts b/src/features/ExternalApi.ts index 913d8e25f3..7943bf8fa6 100644 --- a/src/features/ExternalApi.ts +++ b/src/features/ExternalApi.ts @@ -3,7 +3,6 @@ import * as vscode from "vscode"; import { v4 as uuidv4 } from "uuid"; -import { LanguageClientConsumer } from "../languageClientConsumer"; import { ILogger } from "../logging"; import { SessionManager } from "../session"; @@ -33,14 +32,13 @@ NOTE: At some point, we should release a helper npm package that wraps the API a * Manages session id for you */ -export class ExternalApiFeature extends LanguageClientConsumer implements IPowerShellExtensionClient { +export class ExternalApiFeature implements IPowerShellExtensionClient { private static readonly registeredExternalExtension: Map = new Map(); constructor( private extensionContext: vscode.ExtensionContext, private sessionManager: SessionManager, private logger: ILogger) { - super(); } /* diff --git a/src/features/GetCommands.ts b/src/features/GetCommands.ts index c744b73a89..f85aad28f8 100644 --- a/src/features/GetCommands.ts +++ b/src/features/GetCommands.ts @@ -4,7 +4,6 @@ import * as vscode from "vscode"; import { RequestType0 } from "vscode-languageclient"; import { LanguageClient } from "vscode-languageclient/node"; -import { ILogger } from "../logging"; import { LanguageClientConsumer } from "../languageClientConsumer"; import { getSettings } from "../settings"; @@ -30,7 +29,7 @@ export class GetCommandsFeature extends LanguageClientConsumer { private commandsExplorerProvider: CommandsExplorerProvider; private commandsExplorerTreeView: vscode.TreeView; - constructor(private logger: ILogger) { + constructor() { super(); this.commands = [ vscode.commands.registerCommand("PowerShell.RefreshCommandsExplorer", @@ -56,25 +55,20 @@ export class GetCommandsFeature extends LanguageClientConsumer { } } - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; + public override onLanguageClientSet(_languageClient: LanguageClient): void { if (this.commandsExplorerTreeView.visible) { void vscode.commands.executeCommand("PowerShell.RefreshCommandsExplorer"); } } private async CommandExplorerRefresh(): Promise { - if (this.languageClient === undefined) { - this.logger.writeVerbose(`<${GetCommandsFeature.name}>: Unable to send getCommand request!`); - return; - } - await this.languageClient.sendRequest(GetCommandRequestType).then((result) => { - const exclusions = getSettings().sideBar.CommandExplorerExcludeFilter; - const excludeFilter = exclusions.map((filter: string) => filter.toLowerCase()); - result = result.filter((command) => (!excludeFilter.includes(command.moduleName.toLowerCase()))); - this.commandsExplorerProvider.powerShellCommands = result.map(toCommand); - this.commandsExplorerProvider.refresh(); - }); + const client = await LanguageClientConsumer.getLanguageClient(); + const result = await client.sendRequest(GetCommandRequestType); + const exclusions = getSettings().sideBar.CommandExplorerExcludeFilter; + const excludeFilter = exclusions.map((filter: string) => filter.toLowerCase()); + const filteredResult = result.filter((command) => (!excludeFilter.includes(command.moduleName.toLowerCase()))); + this.commandsExplorerProvider.powerShellCommands = filteredResult.map(toCommand); + this.commandsExplorerProvider.refresh(); } private async InsertCommand(item: { Name: string; }): Promise { @@ -148,5 +142,4 @@ class Command extends vscode.TreeItem { // Returning an empty array because we need to return something. return []; } - } diff --git a/src/features/HelpCompletion.ts b/src/features/HelpCompletion.ts index a28c2ee0e4..426da44df8 100644 --- a/src/features/HelpCompletion.ts +++ b/src/features/HelpCompletion.ts @@ -43,11 +43,11 @@ export class HelpCompletionFeature extends LanguageClientConsumer { this.disposable?.dispose(); } - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; - if (this.helpCompletionProvider) { - this.helpCompletionProvider.languageClient = languageClient; - } + public override onLanguageClientSet(languageClient: LanguageClient): void { + // Our helper class isn't in the session's list of language client + // consumers since we optionally create it, so we have to set it + // manually. + this.helpCompletionProvider?.onLanguageClientSet(languageClient); } public async onEvent(changeEvent: TextDocumentChangeEvent): Promise { @@ -120,14 +120,14 @@ class TriggerFinder { } } -class HelpCompletionProvider { +class HelpCompletionProvider extends LanguageClientConsumer { private triggerFinderHelpComment: TriggerFinder; private lastChangeRange: Range | undefined; private lastDocument: TextDocument | undefined; - private langClient: LanguageClient | undefined; private settings: Settings; constructor() { + super(); this.triggerFinderHelpComment = new TriggerFinder("##"); this.settings = getSettings(); } @@ -136,9 +136,8 @@ class HelpCompletionProvider { return this.triggerFinderHelpComment.found; } - public set languageClient(value: LanguageClient) { - this.langClient = value; - } + // eslint-disable-next-line @typescript-eslint/no-empty-function + public override onLanguageClientSet(_languageClient: LanguageClient): void {} public updateState(document: TextDocument, changeText: string, changeRange: Range): void { this.lastDocument = document; @@ -151,14 +150,15 @@ class HelpCompletionProvider { } public async complete(): Promise { - if (this.langClient === undefined || this.lastChangeRange === undefined || this.lastDocument === undefined) { + if (this.lastChangeRange === undefined || this.lastDocument === undefined) { return; } const triggerStartPos = this.lastChangeRange.start; const doc = this.lastDocument; - const result = await this.langClient.sendRequest(CommentHelpRequestType, { + const client = await LanguageClientConsumer.getLanguageClient(); + const result = await client.sendRequest(CommentHelpRequestType, { documentUri: doc.uri.toString(), triggerPosition: triggerStartPos, blockComment: this.settings.helpCompletion === CommentType.BlockComment, diff --git a/src/features/ISECompatibility.ts b/src/features/ISECompatibility.ts index 270c229ee6..ec56274506 100644 --- a/src/features/ISECompatibility.ts +++ b/src/features/ISECompatibility.ts @@ -25,14 +25,14 @@ export class ISECompatibilityFeature implements vscode.Disposable { { path: "powershell.codeFolding", name: "showLastLine", value: false } ]; - private _commandRegistrations: vscode.Disposable[] = []; - private _iseModeEnabled: boolean; - private _originalSettings: Record = {}; + private commands: vscode.Disposable[] = []; + private iseModeEnabled: boolean; + private originalSettings: Record = {}; constructor() { const testSetting = ISECompatibilityFeature.settings[ISECompatibilityFeature.settings.length - 1]; - this._iseModeEnabled = vscode.workspace.getConfiguration(testSetting.path).get(testSetting.name) === testSetting.value; - this._commandRegistrations = [ + this.iseModeEnabled = vscode.workspace.getConfiguration(testSetting.path).get(testSetting.name) === testSetting.value; + this.commands = [ vscode.commands.registerCommand("PowerShell.EnableISEMode", async () => { await this.EnableISEMode(); }), vscode.commands.registerCommand("PowerShell.DisableISEMode", async () => { await this.DisableISEMode(); }), vscode.commands.registerCommand("PowerShell.ToggleISEMode", async () => { await this.ToggleISEMode(); }) @@ -40,17 +40,17 @@ export class ISECompatibilityFeature implements vscode.Disposable { } public dispose(): void { - for (const command of this._commandRegistrations) { + for (const command of this.commands) { command.dispose(); } } private async EnableISEMode(): Promise { - this._iseModeEnabled = true; + this.iseModeEnabled = true; for (const iseSetting of ISECompatibilityFeature.settings) { try { const config = vscode.workspace.getConfiguration(iseSetting.path); - this._originalSettings[iseSetting.path + iseSetting.name] = config.get(iseSetting.name); + this.originalSettings[iseSetting.path + iseSetting.name] = config.get(iseSetting.name); await config.update(iseSetting.name, iseSetting.value, true); } catch { // The `update` call can fail if the setting doesn't exist. This @@ -66,18 +66,18 @@ export class ISECompatibilityFeature implements vscode.Disposable { } private async DisableISEMode(): Promise { - this._iseModeEnabled = false; + this.iseModeEnabled = false; for (const iseSetting of ISECompatibilityFeature.settings) { const config = vscode.workspace.getConfiguration(iseSetting.path); const currently = config.get(iseSetting.name); if (currently === iseSetting.value) { - await config.update(iseSetting.name, this._originalSettings[iseSetting.path + iseSetting.name], true); + await config.update(iseSetting.name, this.originalSettings[iseSetting.path + iseSetting.name], true); } } } private async ToggleISEMode(): Promise { - if (this._iseModeEnabled) { + if (this.iseModeEnabled) { await this.DisableISEMode(); } else { await this.EnableISEMode(); diff --git a/src/features/NewFileOrProject.ts b/src/features/NewFileOrProject.ts index d42e6e7411..53ed2ae871 100644 --- a/src/features/NewFileOrProject.ts +++ b/src/features/NewFileOrProject.ts @@ -11,50 +11,21 @@ export class NewFileOrProjectFeature extends LanguageClientConsumer { private readonly loadIcon = " $(sync) "; private command: vscode.Disposable; - private waitingForClientToken?: vscode.CancellationTokenSource; constructor(private logger: ILogger) { super(); this.command = - vscode.commands.registerCommand("PowerShell.NewProjectFromTemplate", async () => { - if (!this.languageClient && !this.waitingForClientToken) { - // If PowerShell isn't finished loading yet, show a loading message - // until the LanguageClient is passed on to us - this.waitingForClientToken = new vscode.CancellationTokenSource(); - const response = await vscode.window.showQuickPick( - ["Cancel"], - { placeHolder: "New Project: Please wait, starting PowerShell..." }, - this.waitingForClientToken.token); - - if (response === "Cancel") { - this.clearWaitingToken(); - } - - // Cancel the loading prompt after 60 seconds - setTimeout(() => { - if (this.waitingForClientToken) { - this.clearWaitingToken(); - void this.logger.writeAndShowError("New Project: PowerShell session took too long to start."); - } - }, 60000); - } else { - await this.showProjectTemplates(); - } - }); + vscode.commands.registerCommand( + "PowerShell.NewProjectFromTemplate", + async () => { await this.showProjectTemplates(); }); } public dispose(): void { this.command.dispose(); } - public override setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; - - if (this.waitingForClientToken) { - this.clearWaitingToken(); - void this.showProjectTemplates(); - } - } + // eslint-disable-next-line @typescript-eslint/no-empty-function + public override onLanguageClientSet(_languageClient: LanguageClient): void {} private async showProjectTemplates(includeInstalledModules = false): Promise { const template = await vscode.window.showQuickPick( @@ -74,11 +45,8 @@ export class NewFileOrProjectFeature extends LanguageClientConsumer { } private async getProjectTemplates(includeInstalledModules: boolean): Promise { - if (this.languageClient === undefined) { - return Promise.reject("Language client not defined!"); - } - - const response = await this.languageClient.sendRequest( + const client = await LanguageClientConsumer.getLanguageClient(); + const response = await client.sendRequest( GetProjectTemplatesRequestType, { includeInstalledModules }); @@ -86,37 +54,37 @@ export class NewFileOrProjectFeature extends LanguageClientConsumer { // TODO: Offer to install Plaster void this.logger.writeAndShowError("Plaster is not installed!"); return Promise.reject("Plaster needs to be installed"); - } else { - let templates = response.templates.map( - (template) => { - return { - label: template.title, - description: `v${template.version} by ${template.author}, tags: ${template.tags}`, - detail: template.description, - template, - }; - }); - - if (!includeInstalledModules) { - templates = - [({ - label: this.loadIcon, - description: "Load additional templates from installed modules", - template: undefined, - } as ITemplateQuickPickItem)] - .concat(templates); - } else { - templates = - [({ - label: this.loadIcon, - description: "Refresh template list", - template: undefined, - } as ITemplateQuickPickItem)] - .concat(templates); - } + } - return templates; + let templates = response.templates.map( + (template) => { + return { + label: template.title, + description: `v${template.version} by ${template.author}, tags: ${template.tags}`, + detail: template.description, + template, + }; + }); + + if (!includeInstalledModules) { + templates = + [({ + label: this.loadIcon, + description: "Load additional templates from installed modules", + template: undefined, + } as ITemplateQuickPickItem)] + .concat(templates); + } else { + templates = + [({ + label: this.loadIcon, + description: "Refresh template list", + template: undefined, + } as ITemplateQuickPickItem)] + .concat(templates); } + + return templates; } private async createProjectFromTemplate(template: ITemplateDetails): Promise { @@ -129,10 +97,12 @@ export class NewFileOrProjectFeature extends LanguageClientConsumer { if (destinationPath !== undefined) { await vscode.commands.executeCommand("PowerShell.ShowSessionConsole"); - const result = await this.languageClient?.sendRequest( + const client = await LanguageClientConsumer.getLanguageClient(); + const result = await client.sendRequest( NewProjectFromTemplateRequestType, { templatePath: template.templatePath, destinationPath }); - if (result?.creationSuccessful) { + + if (result.creationSuccessful) { await this.openWorkspacePath(destinationPath); } else { void this.logger.writeAndShowError("Project creation failed, read the Output window for more details."); @@ -161,11 +131,6 @@ export class NewFileOrProjectFeature extends LanguageClientConsumer { vscode.Uri.file(workspacePath), true); } - - private clearWaitingToken(): void { - this.waitingForClientToken?.dispose(); - this.waitingForClientToken = undefined; - } } interface ITemplateQuickPickItem extends vscode.QuickPickItem { diff --git a/src/features/RemoteFiles.ts b/src/features/RemoteFiles.ts index 4c1abdb804..765b6d99a6 100644 --- a/src/features/RemoteFiles.ts +++ b/src/features/RemoteFiles.ts @@ -6,6 +6,7 @@ import path = require("path"); import vscode = require("vscode"); import { NotificationType, TextDocumentIdentifier } from "vscode-languageclient"; import { LanguageClientConsumer } from "../languageClientConsumer"; +import type { LanguageClient } from "vscode-languageclient/node"; // NOTE: The following two DidSaveTextDocument* types will // be removed when #593 gets fixed. @@ -37,8 +38,9 @@ export class RemoteFilesFeature extends LanguageClientConsumer { this.closeRemoteFiles(); this.command = vscode.workspace.onDidSaveTextDocument(async (doc) => { - if (this.isDocumentRemote(doc) && this.languageClient) { - await this.languageClient.sendNotification( + if (this.isDocumentRemote(doc)) { + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendNotification( DidSaveTextDocumentNotificationType, { textDocument: TextDocumentIdentifier.create(doc.uri.toString()), @@ -47,6 +49,9 @@ export class RemoteFilesFeature extends LanguageClientConsumer { }); } + // eslint-disable-next-line @typescript-eslint/no-empty-function + public override onLanguageClientSet(_languageClient: LanguageClient): void {} + public dispose(): void { this.command.dispose(); // Close any leftover remote files before exiting diff --git a/src/features/ShowHelp.ts b/src/features/ShowHelp.ts index d668735455..6e73050924 100644 --- a/src/features/ShowHelp.ts +++ b/src/features/ShowHelp.ts @@ -4,6 +4,7 @@ import vscode = require("vscode"); import { NotificationType } from "vscode-languageclient"; import { LanguageClientConsumer } from "../languageClientConsumer"; +import type { LanguageClient } from "vscode-languageclient/node"; // eslint-disable-next-line @typescript-eslint/no-empty-interface interface IShowHelpNotificationArguments { @@ -30,13 +31,18 @@ export class ShowHelpFeature extends LanguageClientConsumer { const cwr = doc.getWordRangeAtPosition(selection.active); const text = doc.getText(cwr); - await this.languageClient?.sendNotification(ShowHelpNotificationType, { text }); + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendNotification(ShowHelpNotificationType, { text }); } else { - await this.languageClient?.sendNotification(ShowHelpNotificationType, { text: item.Name }); + const client = await LanguageClientConsumer.getLanguageClient(); + await client.sendNotification(ShowHelpNotificationType, { text: item.Name }); } }); } + // eslint-disable-next-line @typescript-eslint/no-empty-function + public override onLanguageClientSet(_languageClient: LanguageClient): void {} + public dispose(): void { this.command.dispose(); } diff --git a/src/languageClientConsumer.ts b/src/languageClientConsumer.ts index fb5d1e84b0..d4acdbd4b6 100644 --- a/src/languageClientConsumer.ts +++ b/src/languageClientConsumer.ts @@ -1,28 +1,66 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { window } from "vscode"; +import { ProgressLocation, window } from "vscode"; import { LanguageClient } from "vscode-languageclient/node"; export abstract class LanguageClientConsumer { + private static languageClientPromise?: Promise; + private static getLanguageClientResolve?: (value: LanguageClient) => void; - private _languageClient: LanguageClient | undefined; + // Implementations of this class must override this method to register their + // handlers, as its called whenever the client is restarted / replaced. + public abstract onLanguageClientSet(languageClient: LanguageClient): void; - public setLanguageClient(languageClient: LanguageClient): void { - this.languageClient = languageClient; + // This is called in the session manager when the client is started (so we + // can wait for that). It's what actually resolves the promise. + public static onLanguageClientStarted(languageClient: LanguageClient): void { + // It should have been created earlier, but if not, create and resolve it. + this.languageClientPromise ??= Promise.resolve(languageClient); + this.getLanguageClientResolve?.(languageClient); } - abstract dispose(): void; + // This is called in the session manager when the client exits so we can + // make a new promise. + public static onLanguageClientExited(): void { + this.languageClientPromise = undefined; + this.getLanguageClientResolve = undefined; + } - public get languageClient(): LanguageClient | undefined { - if (!this._languageClient) { - // TODO: Plumb through the logger. - void window.showInformationMessage("PowerShell extension has not finished starting up yet. Please try again in a few moments."); - } - return this._languageClient; + // We should have a promise as defined in resetLanguageClient, but if we + // don't, create it. + public static async getLanguageClient(): Promise { + // If it hasn't been created or was rejected, recreate it. + LanguageClientConsumer.languageClientPromise?.catch(() => { + LanguageClientConsumer.languageClientPromise = undefined; + }); + LanguageClientConsumer.languageClientPromise ??= LanguageClientConsumer.createLanguageClientPromise(); + return LanguageClientConsumer.languageClientPromise; } - public set languageClient(value: LanguageClient | undefined) { - this._languageClient = value; + // This waits for the language client to start and shows a cancellable + // loading message. (It just wrap the static method below.) + private static async createLanguageClientPromise(): Promise { + return window.withProgress( + { + location: ProgressLocation.Notification, + title: "Please wait, starting PowerShell Extension Terminal...", + cancellable: true + }, + (_progress, token) => { + token.onCancellationRequested(() => { + void window.showErrorMessage("Cancelled PowerShell Extension Terminal start-up."); + }); + + // The real promise! + return new Promise( + (resolve, reject) => { + // Store the resolve function to be called in resetLanguageClient. + LanguageClientConsumer.getLanguageClientResolve = resolve; + // Reject the promise if the operation is cancelled. + token.onCancellationRequested(() => { reject(); }); + } + ); + }); } } diff --git a/src/main.ts b/src/main.ts index 9cbaeac5af..82a4353e63 100644 --- a/src/main.ts +++ b/src/main.ts @@ -18,7 +18,6 @@ import { ISECompatibilityFeature } from "./features/ISECompatibility"; import { NewFileOrProjectFeature } from "./features/NewFileOrProject"; import { OpenInISEFeature } from "./features/OpenInISE"; import { PesterTestsFeature } from "./features/PesterTests"; -import { PickPSHostProcessFeature, PickRunspaceFeature } from "./features/DebugSession"; import { RemoteFilesFeature } from "./features/RemoteFiles"; import { ShowHelpFeature } from "./features/ShowHelp"; import { SpecifyScriptArgsFeature } from "./features/DebugSession"; @@ -147,16 +146,13 @@ export async function activate(context: vscode.ExtensionContext): Promise { // Clean up all extension features - for (const languageClientConsumer of languageClientConsumers) { - languageClientConsumer.dispose(); - } - for (const commandRegistration of commandRegistrations) { commandRegistration.dispose(); } diff --git a/src/session.ts b/src/session.ts index df896cfbc5..866117316a 100644 --- a/src/session.ts +++ b/src/session.ts @@ -531,6 +531,8 @@ export class SessionManager implements Middleware { languageServerProcess.onExited( async () => { + LanguageClientConsumer.onLanguageClientExited(); + if (this.sessionStatus === SessionStatus.Running || this.sessionStatus === SessionStatus.Busy) { this.setSessionStatus("Session Exited!", SessionStatus.Failed); @@ -661,7 +663,7 @@ export class SessionManager implements Middleware { // so that they can register their message handlers // before the connection is established. for (const consumer of this.languageClientConsumers) { - consumer.setLanguageClient(languageClient); + consumer.onLanguageClientSet(languageClient); } this.registeredHandlers = [ @@ -684,6 +686,7 @@ export class SessionManager implements Middleware { try { await languageClient.start(); + LanguageClientConsumer.onLanguageClientStarted(languageClient); } catch (err) { void this.setSessionFailedOpenBug("Language client failed to start: " + (err instanceof Error ? err.message : "unknown")); } diff --git a/test/features/DebugSession.test.ts b/test/features/DebugSession.test.ts index 60e18021cc..24e8e7d51f 100644 --- a/test/features/DebugSession.test.ts +++ b/test/features/DebugSession.test.ts @@ -50,6 +50,8 @@ describe("DebugSessionFeature", () => { }); beforeEach(() => { + // Because we recreate DebugSessionFeature constantly, we need to avoid registering the same commands over and over. + Sinon.stub(commands, "registerCommand").returns(Disposable.create(() => {"Stubbed";})); registerProviderStub = Sinon.stub(debug, "registerDebugConfigurationProvider").returns(Disposable.create(() => {"Stubbed";})); registerFactoryStub = Sinon.stub(debug, "registerDebugAdapterDescriptorFactory").returns(Disposable.create(() => {"Stubbed";})); }); @@ -57,6 +59,7 @@ describe("DebugSessionFeature", () => { afterEach(() => { Sinon.restore(); }); + describe("Constructor", () => { it("Registers debug configuration provider and factory", () => { const context = stubInterface({ @@ -232,14 +235,17 @@ describe("DebugSessionFeature", () => { version: "7.2.3" }) ); - const executeCommandStub = Sinon.stub(commands, "executeCommand").resolves(7357); - - const actual = await createDebugSessionFeatureStub({ + const debugSessionFeatureStub = createDebugSessionFeatureStub({ sessionManager: sessionManager - }).resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pickPSHostProcessStub = Sinon.stub(debugSessionFeatureStub , "pickPSHostProcess" as any).resolves(7357); + + const actual = await debugSessionFeatureStub.resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); assert.equal(actual!.processId, TEST_NUMBER); - assert.ok(executeCommandStub.calledOnceWith("PowerShell.PickPSHostProcess")); + assert.ok(pickPSHostProcessStub.calledOnce); }); it("Attach: Exits if process was not selected from the picker", async () => { @@ -254,14 +260,17 @@ describe("DebugSessionFeature", () => { version: "7.2.3" }) ); - const executeCommandStub = Sinon.stub(commands, "executeCommand").resolves(undefined); - - const actual = await createDebugSessionFeatureStub({ + const debugSessionFeatureStub = createDebugSessionFeatureStub({ sessionManager: sessionManager - }).resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pickPSHostProcessStub = Sinon.stub(debugSessionFeatureStub, "pickPSHostProcess" as any).resolves(undefined); + + const actual = await debugSessionFeatureStub.resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); assert.equal(actual, undefined); - assert.ok(executeCommandStub.calledOnceWith("PowerShell.PickPSHostProcess")); + assert.ok(pickPSHostProcessStub.calledOnce); }); it("Attach: Prompts for Runspace if not specified", async () => { @@ -275,14 +284,17 @@ describe("DebugSessionFeature", () => { version: "7.2.3" }) ); - const executeCommandStub = Sinon.stub(commands, "executeCommand").resolves(TEST_NUMBER); - - const actual = await createDebugSessionFeatureStub({ + const debugSessionFeatureStub = createDebugSessionFeatureStub({ sessionManager: sessionManager - }).resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pickRunspaceStub = Sinon.stub(debugSessionFeatureStub, "pickRunspace" as any).resolves(TEST_NUMBER); + + const actual = await debugSessionFeatureStub.resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); assert.equal(actual!.runspaceId, TEST_NUMBER); - assert.ok(executeCommandStub.calledOnceWith("PowerShell.PickRunspace", TEST_NUMBER)); + assert.ok(pickRunspaceStub.calledOnceWith(TEST_NUMBER)); }); it("Attach: Exits if runspace was not selected from the picker", async () => { @@ -296,13 +308,16 @@ describe("DebugSessionFeature", () => { version: "7.2.3" }) ); - const executeCommandStub = Sinon.stub(commands, "executeCommand").resolves(undefined); - - const actual = await createDebugSessionFeatureStub({ + const debugSessionFeatureStub = createDebugSessionFeatureStub({ sessionManager: sessionManager - }).resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pickRunspaceStub = Sinon.stub(debugSessionFeatureStub, "pickRunspace" as any).resolves(undefined); + + const actual = await debugSessionFeatureStub.resolveDebugConfigurationWithSubstitutedVariables(undefined, attachConfig); assert.equal(actual, undefined); - assert.ok(executeCommandStub.calledOnceWith("PowerShell.PickRunspace", TEST_NUMBER)); + assert.ok(pickRunspaceStub.calledOnceWith(TEST_NUMBER)); }); it("Starts dotnet attach debug session with default config", async () => { @@ -394,7 +409,6 @@ describe("DebugSessionFeature", () => { attachConfig.dotnetDebuggerConfigName = foundDotnetConfig.name; const debugSessionFeature = createDebugSessionFeatureStub({}); - // The any is necessary to stub a private method // eslint-disable-next-line @typescript-eslint/no-explicit-any Sinon.stub(debugSessionFeature, "getLaunchConfigurations" as any).returns(candidateDotnetConfigs);