From 3043a3381d859367b300dee26456a10cdad4e618 Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Thu, 10 Aug 2023 17:22:49 -0700 Subject: [PATCH] Fix race condition with displaying PowerShell name on icon Refactors the logic a bit so the that Language Status Icon has the correct details when switching sessions. Also delete superfluous (and apparently broken) migration code. --- src/session.ts | 67 ++++++++++++++++++------------------------------- src/settings.ts | 2 -- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/src/session.ts b/src/session.ts index 0d3cc093d0..f522ce5fbb 100644 --- a/src/session.ts +++ b/src/session.ts @@ -147,7 +147,7 @@ export class SessionManager implements Middleware { // The `exeNameOverride` is used by `restartSession` to override ANY other setting. // We've made this function idempotent, so it can used to ensure the session has started. - public async start(exeNameOverride?: string): Promise { + public async start(): Promise { switch (this.sessionStatus) { case SessionStatus.NotStarted: // Go ahead and start. @@ -177,20 +177,16 @@ export class SessionManager implements Middleware { break; } + // This status needs to be set immediately so the above check works this.setSessionStatus("Starting...", SessionStatus.Starting); + this.startCancellationTokenSource = new vscode.CancellationTokenSource(); const cancellationToken = this.startCancellationTokenSource.token; - if (exeNameOverride != undefined) { - this.logger.writeVerbose(`Starting with executable overriden to: ${exeNameOverride}`); - this.sessionSettings.powerShellDefaultVersion = exeNameOverride; - } - // Create a folder for the session files. await vscode.workspace.fs.createDirectory(this.sessionsFolder); // Migrate things. - await this.promptPowerShellExeSettingsCleanup(); await this.migrateWhitespaceAroundPipeSetting(); // Find the PowerShell executable to use for the server. @@ -205,6 +201,8 @@ export class SessionManager implements Middleware { return; } + // Refresh the status with the found executable details. + this.refreshSessionStatus(); this.logger.write(`Starting '${this.PowerShellExeDetails.displayName}' at: ${this.PowerShellExeDetails.exePath}`); // Start the server. @@ -274,7 +272,6 @@ export class SessionManager implements Middleware { this.startCancellationTokenSource?.dispose(); this.startCancellationTokenSource = undefined; this.sessionDetails = undefined; - this.versionDetails = undefined; this.setSessionStatus("Not Started", SessionStatus.NotStarted); } @@ -286,7 +283,16 @@ export class SessionManager implements Middleware { // Re-load the settings. this.sessionSettings = getSettings(); - await this.start(exeNameOverride); + if (exeNameOverride) { + // Reset the version and PowerShell details since we're launching a + // new executable. + this.logger.writeVerbose(`Starting with executable overriden to: ${exeNameOverride}`); + this.sessionSettings.powerShellDefaultVersion = exeNameOverride; + this.versionDetails = undefined; + this.PowerShellExeDetails = undefined; + } + + await this.start(); } public getSessionDetails(): IEditorServicesSessionDetails | undefined { @@ -432,38 +438,6 @@ export class SessionManager implements Middleware { } } - // TODO: Remove this migration code. - private async promptPowerShellExeSettingsCleanup(): Promise { - if (this.sessionSettings.powerShellExePath === "") { - return; - } - - this.logger.writeWarning("Deprecated setting: powerShellExePath"); - let warningMessage = "The 'powerShell.powerShellExePath' setting is no longer used. "; - warningMessage += this.sessionSettings.powerShellDefaultVersion - ? "We can automatically remove it for you." - : "We can remove it from your settings and prompt you for which PowerShell you want to use."; - - const choice = await vscode.window.showWarningMessage(warningMessage, "Let's do it!"); - - if (choice === undefined) { - // They hit the 'x' to close the dialog. - return; - } - - this.suppressRestartPrompt = true; - try { - await changeSetting("powerShellExePath", undefined, true, this.logger); - } finally { - this.suppressRestartPrompt = false; - } - - // Show the session menu at the end if they don't have a PowerShellDefaultVersion. - if (this.sessionSettings.powerShellDefaultVersion === "") { - await vscode.commands.executeCommand(this.ShowSessionMenuCommandName); - } - } - private async onConfigurationUpdated(): Promise { const settings = getSettings(); this.logger.updateLogLevel(settings.developer.editorServicesLogLevel); @@ -524,7 +498,6 @@ export class SessionManager implements Middleware { break; } } - } foundPowerShell = defaultPowerShell ?? await powershellExeFinder.getFirstAvailablePowerShellInstallation(); if (wantedName !== "" && defaultPowerShell === undefined && foundPowerShell !== undefined) { @@ -846,9 +819,12 @@ Type 'help' to get help. const semver = new SemVer(this.versionDetails.version); this.languageStatusItem.text += ` ${semver.major}.${semver.minor}`; this.languageStatusItem.detail += ` ${this.versionDetails.commit} (${this.versionDetails.architecture.toLowerCase()})`; - } else if (this.PowerShellExeDetails?.displayName) { // In case it didn't start. + } else if (this.PowerShellExeDetails?.displayName) { // When it hasn't started yet. this.languageStatusItem.text += ` ${this.PowerShellExeDetails.displayName}`; this.languageStatusItem.detail += ` at '${this.PowerShellExeDetails.exePath}'`; + } else if (this.sessionSettings.powerShellDefaultVersion) { // When it hasn't been found yet. + this.languageStatusItem.text += ` ${this.sessionSettings.powerShellDefaultVersion}`; + this.languageStatusItem.detail = `Looking for '${this.sessionSettings.powerShellDefaultVersion}'...`; } if (detail) { @@ -877,6 +853,11 @@ Type 'help' to get help. } } + // Refreshes the Language Status Item details with ehe same status. + private refreshSessionStatus(): void { + this.setSessionStatus("", this.sessionStatus); + } + private setSessionRunningStatus(): void { this.setSessionStatus("", SessionStatus.Running); } diff --git a/src/settings.ts b/src/settings.ts index 8b838f5a4d..f984d52c64 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -22,8 +22,6 @@ class PartialSettings { } export class Settings extends PartialSettings { powerShellAdditionalExePaths: PowerShellAdditionalExePathSettings = {}; powerShellDefaultVersion = ""; - // This setting is no longer used but is here to assist in cleaning up the users settings. - powerShellExePath = ""; promptToUpdatePowerShell = true; suppressAdditionalExeNotFoundWarning = false; startAsLoginShell = new StartAsLoginShellSettings();