Skip to content

Commit 24d6dcb

Browse files
committed
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.
1 parent ee21f8c commit 24d6dcb

File tree

2 files changed

+24
-45
lines changed

2 files changed

+24
-45
lines changed

src/session.ts

+24-43
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export class SessionManager implements Middleware {
147147

148148
// The `exeNameOverride` is used by `restartSession` to override ANY other setting.
149149
// We've made this function idempotent, so it can used to ensure the session has started.
150-
public async start(exeNameOverride?: string): Promise<void> {
150+
public async start(): Promise<void> {
151151
switch (this.sessionStatus) {
152152
case SessionStatus.NotStarted:
153153
// Go ahead and start.
@@ -177,20 +177,16 @@ export class SessionManager implements Middleware {
177177
break;
178178
}
179179

180+
// This status needs to be set immediately so the above check works
180181
this.setSessionStatus("Starting...", SessionStatus.Starting);
182+
181183
this.startCancellationTokenSource = new vscode.CancellationTokenSource();
182184
const cancellationToken = this.startCancellationTokenSource.token;
183185

184-
if (exeNameOverride != undefined) {
185-
this.logger.writeVerbose(`Starting with executable overriden to: ${exeNameOverride}`);
186-
this.sessionSettings.powerShellDefaultVersion = exeNameOverride;
187-
}
188-
189186
// Create a folder for the session files.
190187
await vscode.workspace.fs.createDirectory(this.sessionsFolder);
191188

192189
// Migrate things.
193-
await this.promptPowerShellExeSettingsCleanup();
194190
await this.migrateWhitespaceAroundPipeSetting();
195191

196192
// Find the PowerShell executable to use for the server.
@@ -205,6 +201,8 @@ export class SessionManager implements Middleware {
205201
return;
206202
}
207203

204+
// Refresh the status with the found executable details.
205+
this.refreshSessionStatus();
208206
this.logger.write(`Starting '${this.PowerShellExeDetails.displayName}' at: ${this.PowerShellExeDetails.exePath}`);
209207

210208
// Start the server.
@@ -274,7 +272,6 @@ export class SessionManager implements Middleware {
274272
this.startCancellationTokenSource?.dispose();
275273
this.startCancellationTokenSource = undefined;
276274
this.sessionDetails = undefined;
277-
this.versionDetails = undefined;
278275

279276
this.setSessionStatus("Not Started", SessionStatus.NotStarted);
280277
}
@@ -286,7 +283,16 @@ export class SessionManager implements Middleware {
286283
// Re-load the settings.
287284
this.sessionSettings = getSettings();
288285

289-
await this.start(exeNameOverride);
286+
if (exeNameOverride) {
287+
// Reset the version and PowerShell details since we're launching a
288+
// new executable.
289+
this.logger.writeVerbose(`Starting with executable overriden to: ${exeNameOverride}`);
290+
this.sessionSettings.powerShellDefaultVersion = exeNameOverride;
291+
this.versionDetails = undefined;
292+
this.PowerShellExeDetails = undefined;
293+
}
294+
295+
await this.start();
290296
}
291297

292298
public getSessionDetails(): IEditorServicesSessionDetails | undefined {
@@ -432,38 +438,6 @@ export class SessionManager implements Middleware {
432438
}
433439
}
434440

435-
// TODO: Remove this migration code.
436-
private async promptPowerShellExeSettingsCleanup(): Promise<void> {
437-
if (this.sessionSettings.powerShellExePath === "") {
438-
return;
439-
}
440-
441-
this.logger.writeWarning("Deprecated setting: powerShellExePath");
442-
let warningMessage = "The 'powerShell.powerShellExePath' setting is no longer used. ";
443-
warningMessage += this.sessionSettings.powerShellDefaultVersion
444-
? "We can automatically remove it for you."
445-
: "We can remove it from your settings and prompt you for which PowerShell you want to use.";
446-
447-
const choice = await vscode.window.showWarningMessage(warningMessage, "Let's do it!");
448-
449-
if (choice === undefined) {
450-
// They hit the 'x' to close the dialog.
451-
return;
452-
}
453-
454-
this.suppressRestartPrompt = true;
455-
try {
456-
await changeSetting("powerShellExePath", undefined, true, this.logger);
457-
} finally {
458-
this.suppressRestartPrompt = false;
459-
}
460-
461-
// Show the session menu at the end if they don't have a PowerShellDefaultVersion.
462-
if (this.sessionSettings.powerShellDefaultVersion === "") {
463-
await vscode.commands.executeCommand(this.ShowSessionMenuCommandName);
464-
}
465-
}
466-
467441
private async onConfigurationUpdated(): Promise<void> {
468442
const settings = getSettings();
469443
this.logger.updateLogLevel(settings.developer.editorServicesLogLevel);
@@ -524,7 +498,6 @@ export class SessionManager implements Middleware {
524498
break;
525499
}
526500
}
527-
528501
}
529502
foundPowerShell = defaultPowerShell ?? await powershellExeFinder.getFirstAvailablePowerShellInstallation();
530503
if (wantedName !== "" && defaultPowerShell === undefined && foundPowerShell !== undefined) {
@@ -846,9 +819,12 @@ Type 'help' to get help.
846819
const semver = new SemVer(this.versionDetails.version);
847820
this.languageStatusItem.text += ` ${semver.major}.${semver.minor}`;
848821
this.languageStatusItem.detail += ` ${this.versionDetails.commit} (${this.versionDetails.architecture.toLowerCase()})`;
849-
} else if (this.PowerShellExeDetails?.displayName) { // In case it didn't start.
822+
} else if (this.PowerShellExeDetails?.displayName) { // When it hasn't started yet.
850823
this.languageStatusItem.text += ` ${this.PowerShellExeDetails.displayName}`;
851824
this.languageStatusItem.detail += ` at '${this.PowerShellExeDetails.exePath}'`;
825+
} else if (this.sessionSettings.powerShellDefaultVersion) { // When it hasn't been found yet.
826+
this.languageStatusItem.text += ` ${this.sessionSettings.powerShellDefaultVersion}`;
827+
this.languageStatusItem.detail = `Looking for '${this.sessionSettings.powerShellDefaultVersion}'...`;
852828
}
853829

854830
if (detail) {
@@ -877,6 +853,11 @@ Type 'help' to get help.
877853
}
878854
}
879855

856+
// Refreshes the Language Status Item details with ehe same status.
857+
private refreshSessionStatus(): void {
858+
this.setSessionStatus("", this.sessionStatus);
859+
}
860+
880861
private setSessionRunningStatus(): void {
881862
this.setSessionStatus("", SessionStatus.Running);
882863
}

src/settings.ts

-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ class PartialSettings { }
2222
export class Settings extends PartialSettings {
2323
powerShellAdditionalExePaths: PowerShellAdditionalExePathSettings = {};
2424
powerShellDefaultVersion = "";
25-
// This setting is no longer used but is here to assist in cleaning up the users settings.
26-
powerShellExePath = "";
2725
promptToUpdatePowerShell = true;
2826
suppressAdditionalExeNotFoundWarning = false;
2927
startAsLoginShell = new StartAsLoginShellSettings();

0 commit comments

Comments
 (0)