fix: some bugs with terminal shell activation

Author: karthiknadig

src/common/window.apis.ts

@@ -29,6 +29,7 @@ import {
     TextEditor,
     Uri,
     window,
+    WindowState,
 } from 'vscode';
 import { createDeferred } from './utils/deferred';
 
@@ -347,3 +348,11 @@ export function createLogOutputChannel(name: string): LogOutputChannel {
 export function registerFileDecorationProvider(provider: FileDecorationProvider): Disposable {
     return window.registerFileDecorationProvider(provider);
 }
+
+export function onDidChangeWindowState(
+    listener: (e: WindowState) => any,
+    thisArgs?: any,
+    disposables?: Disposable[],
+): Disposable {
+    return window.onDidChangeWindowState(listener, thisArgs, disposables);
+}

src/features/terminal/shellStartupSetupHandlers.ts

@@ -1,8 +1,8 @@
-import { l10n } from 'vscode';
+import { l10n, ProgressLocation } from 'vscode';
 import { executeCommand } from '../../common/command.api';
 import { Common, ShellStartupActivationStrings } from '../../common/localize';
 import { traceInfo, traceVerbose } from '../../common/logging';
-import { showErrorMessage, showInformationMessage } from '../../common/window.apis';
+import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis';
 import { ShellScriptEditState, ShellStartupScriptProvider } from './shells/startupProvider';
 import { getAutoActivationType, setAutoActivationType } from './utils';
 
@@ -23,8 +23,16 @@ export async function handleSettingUpShellProfile(
 
     if (response === Common.yes) {
         traceVerbose(`User chose to set up shell profiles for ${shells} shells`);
-        const states = (await Promise.all(providers.map((provider) => provider.setupScripts()))).filter(
-            (state) => state !== ShellScriptEditState.NotInstalled,
+        const states = await withProgress(
+            {
+                location: ProgressLocation.Notification,
+                title: l10n.t('Setting up shell profiles for {0}', shells),
+            },
+            async () => {
+                return (await Promise.all(providers.map((provider) => provider.setupScripts()))).filter(
+                    (state) => state !== ShellScriptEditState.NotInstalled,
+                );
+            },
         );
         if (states.every((state) => state === ShellScriptEditState.Edited)) {
             setImmediate(async () => {

src/features/terminal/shells/bash/bashEnvs.ts

@@ -14,6 +14,10 @@ export class BashEnvsProvider implements ShellEnvsProvider {
             const bashActivation = getShellActivationCommand(this.shellType, env);
             if (bashActivation) {
                 const command = getShellCommandAsString(this.shellType, bashActivation);
+                const v = collection.get(BASH_ENV_KEY);
+                if (v?.value === command) {
+                    return;
+                }
                 collection.replace(BASH_ENV_KEY, command);
             } else {
                 collection.delete(BASH_ENV_KEY);
@@ -49,23 +53,27 @@ export class BashEnvsProvider implements ShellEnvsProvider {
 
 export class ZshEnvsProvider implements ShellEnvsProvider {
     public readonly shellType: string = ShellConstants.ZSH;
-    updateEnvVariables(envVars: EnvironmentVariableCollection, env: PythonEnvironment): void {
+    updateEnvVariables(collection: EnvironmentVariableCollection, env: PythonEnvironment): void {
         try {
             const zshActivation = getShellActivationCommand(this.shellType, env);
             if (zshActivation) {
                 const command = getShellCommandAsString(this.shellType, zshActivation);
-                envVars.replace(ZSH_ENV_KEY, command);
+                const v = collection.get(ZSH_ENV_KEY);
+                if (v?.value === command) {
+                    return;
+                }
+                collection.replace(ZSH_ENV_KEY, command);
             } else {
-                envVars.delete(ZSH_ENV_KEY);
+                collection.delete(ZSH_ENV_KEY);
             }
         } catch (err) {
             traceError('Failed to update env variables for zsh', err);
-            envVars.delete(ZSH_ENV_KEY);
+            collection.delete(ZSH_ENV_KEY);
         }
     }
 
-    removeEnvVariables(envVars: EnvironmentVariableCollection): void {
-        envVars.delete(ZSH_ENV_KEY);
+    removeEnvVariables(collection: EnvironmentVariableCollection): void {
+        collection.delete(ZSH_ENV_KEY);
     }
 
     getEnvVariables(env?: PythonEnvironment): Map<string, string | undefined> | undefined {

src/features/terminal/shells/cmd/cmdEnvs.ts

@@ -13,6 +13,10 @@ export class CmdEnvsProvider implements ShellEnvsProvider {
             const cmdActivation = getShellActivationCommand(this.shellType, env);
             if (cmdActivation) {
                 const command = getShellCommandAsString(this.shellType, cmdActivation);
+                const v = collection.get(CMD_ENV_KEY);
+                if (v?.value === command) {
+                    return;
+                }
                 collection.replace(CMD_ENV_KEY, command);
             } else {
                 collection.delete(CMD_ENV_KEY);

src/features/terminal/shells/fish/fishEnvs.ts

@@ -13,6 +13,10 @@ export class FishEnvsProvider implements ShellEnvsProvider {
             const fishActivation = getShellActivationCommand(this.shellType, env);
             if (fishActivation) {
                 const command = getShellCommandAsString(this.shellType, fishActivation);
+                const v = collection.get(FISH_ENV_KEY);
+                if (v?.value === command) {
+                    return;
+                }
                 collection.replace(FISH_ENV_KEY, command);
             } else {
                 collection.delete(FISH_ENV_KEY);

src/features/terminal/shells/providers.ts

@@ -7,19 +7,25 @@ import { CmdStartupProvider } from './cmd/cmdStartup';
 import { FishEnvsProvider } from './fish/fishEnvs';
 import { FishStartupProvider } from './fish/fishStartup';
 import { PowerShellEnvsProvider } from './pwsh/pwshEnvs';
-import { PowerShellClassicStartupProvider, PwshStartupProvider } from './pwsh/pwshStartup';
+import { PwshStartupProvider } from './pwsh/pwshStartup';
 import { ShellEnvsProvider, ShellStartupScriptProvider } from './startupProvider';
 
 export function createShellStartupProviders(): ShellStartupScriptProvider[] {
     if (isWindows()) {
         return [
-            new PowerShellClassicStartupProvider(),
-            new PwshStartupProvider(),
+            // PowerShell classic is the default on Windows, so it is included here explicitly.
+            // pwsh is the new PowerShell Core, which is cross-platform and preferred.
+            new PwshStartupProvider([ShellConstants.PWSH, 'powershell']),
             new GitBashStartupProvider(),
             new CmdStartupProvider(),
         ];
     }
-    return [new PwshStartupProvider(), new BashStartupProvider(), new FishStartupProvider(), new ZshStartupProvider()];
+    return [
+        new PwshStartupProvider([ShellConstants.PWSH]),
+        new BashStartupProvider(),
+        new FishStartupProvider(),
+        new ZshStartupProvider(),
+    ];
 }
 
 export function createShellEnvProviders(): ShellEnvsProvider[] {

src/features/terminal/shells/pwsh/pwshEnvs.ts

@@ -14,6 +14,10 @@ export class PowerShellEnvsProvider implements ShellEnvsProvider {
             const pwshActivation = getShellActivationCommand(this.shellType, env);
             if (pwshActivation) {
                 const command = getShellCommandAsString(this.shellType, pwshActivation);
+                const v = collection.get(POWERSHELL_ENV_KEY);
+                if (v?.value === command) {
+                    return;
+                }
                 collection.replace(POWERSHELL_ENV_KEY, command);
             } else {
                 collection.delete(POWERSHELL_ENV_KEY);

src/features/terminal/shells/pwsh/pwshStartup.ts

@@ -7,6 +7,7 @@ import { isWindows } from '../../../../common/utils/platformUtils';
 import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
 import { runCommand } from '../utils';
 
+import assert from 'assert';
 import { getGlobalPersistentState } from '../../../../common/persistentState';
 import { ShellConstants } from '../../../common/shellConstants';
 import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
@@ -185,130 +186,134 @@ async function removePowerShellStartup(shell: string, profile: string): Promise<
     }
 }
 
-export class PowerShellClassicStartupProvider implements ShellStartupScriptProvider {
-    public readonly name: string = 'PowerShell5';
-    public readonly shellType: string = 'powershell';
-    private _isInstalled: boolean | undefined;
+type PowerShellType = 'powershell' | 'pwsh';
 
-    private async checkInstallation(): Promise<boolean> {
-        if (this._isInstalled === undefined) {
-            this._isInstalled = await isPowerShellInstalled('powershell');
-        }
-        return this._isInstalled;
-    }
-    async isSetup(): Promise<ShellSetupState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
-            return ShellSetupState.NotInstalled;
-        }
+export class PwshStartupProvider implements ShellStartupScriptProvider {
+    public readonly name: string = 'PowerShell';
+    public readonly shellType: string = ShellConstants.PWSH;
 
-        try {
-            const profile = await getProfileForShell('powershell');
-            const isSetup = await isPowerShellStartupSetup('powershell', profile);
-            return isSetup ? ShellSetupState.Setup : ShellSetupState.NotSetup;
-        } catch (err) {
-            traceError('Failed to check if PowerShell startup is setup', err);
-        }
-        return ShellSetupState.NotSetup;
+    private _isPwshInstalled: boolean | undefined;
+    private _isPs5Installed: boolean | undefined;
+    private _supportedShells: PowerShellType[];
+
+    constructor(supportedShells: PowerShellType[]) {
+        assert(supportedShells.length > 0, 'At least one PowerShell shell must be supported');
+        this._supportedShells = supportedShells;
     }
 
-    async setupScripts(): Promise<ShellScriptEditState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
-            return ShellScriptEditState.NotInstalled;
-        }
+    private async checkInstallations(): Promise<Map<PowerShellType, boolean>> {
+        const results = new Map<PowerShellType, boolean>();
 
-        try {
-            const profile = await getProfileForShell('powershell');
-            const success = await setupPowerShellStartup('powershell', profile);
-            return success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited;
-        } catch (err) {
-            traceError('Failed to setup PowerShell startup', err);
-        }
-        return ShellScriptEditState.NotEdited;
+        await Promise.all(
+            this._supportedShells.map(async (shell) => {
+                if (shell === 'pwsh' && this._isPwshInstalled !== undefined) {
+                    results.set(shell, this._isPwshInstalled);
+                } else if (shell === 'powershell' && this._isPs5Installed !== undefined) {
+                    results.set(shell, this._isPs5Installed);
+                } else {
+                    const isInstalled = await isPowerShellInstalled(shell);
+                    if (shell === 'pwsh') {
+                        this._isPwshInstalled = isInstalled;
+                    } else {
+                        this._isPs5Installed = isInstalled;
+                    }
+                    results.set(shell, isInstalled);
+                }
+            }),
+        );
+
+        return results;
     }
 
-    async teardownScripts(): Promise<ShellScriptEditState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
-            return ShellScriptEditState.NotInstalled;
-        }
+    async isSetup(): Promise<ShellSetupState> {
+        const installations = await this.checkInstallations();
 
-        try {
-            const profile = await getProfileForShell('powershell');
-            const success = await removePowerShellStartup('powershell', profile);
-            return success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited;
-        } catch (err) {
-            traceError('Failed to remove PowerShell startup', err);
+        if (Array.from(installations.values()).every((installed) => !installed)) {
+            return ShellSetupState.NotInstalled;
         }
-        return ShellScriptEditState.NotEdited;
-    }
-    async clearCache(): Promise<void> {
-        await clearPwshCache('powershell');
-    }
-}
-
-export class PwshStartupProvider implements ShellStartupScriptProvider {
-    public readonly name: string = 'PowerShell';
-    public readonly shellType: string = ShellConstants.PWSH;
 
-    private _isInstalled: boolean | undefined;
+        const results: ShellSetupState[] = [];
+        for (const [shell, installed] of installations.entries()) {
+            if (!installed) {
+                continue;
+            }
 
-    private async checkInstallation(): Promise<boolean> {
-        if (this._isInstalled === undefined) {
-            this._isInstalled = await isPowerShellInstalled('pwsh');
+            try {
+                const profile = await getProfileForShell(shell);
+                const isSetup = await isPowerShellStartupSetup(shell, profile);
+                results.push(isSetup ? ShellSetupState.Setup : ShellSetupState.NotSetup);
+            } catch (err) {
+                traceError(`Failed to check if ${shell} startup is setup`, err);
+                results.push(ShellSetupState.NotSetup);
+            }
         }
-        return this._isInstalled;
-    }
 
-    async isSetup(): Promise<ShellSetupState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
-            return ShellSetupState.NotInstalled;
+        if (results.includes(ShellSetupState.NotSetup)) {
+            return ShellSetupState.NotSetup;
         }
 
-        try {
-            const profile = await getProfileForShell('pwsh');
-            const isSetup = await isPowerShellStartupSetup('pwsh', profile);
-            return isSetup ? ShellSetupState.Setup : ShellSetupState.NotSetup;
-        } catch (err) {
-            traceError('Failed to check if PowerShell startup is setup', err);
-        }
-        return ShellSetupState.NotSetup;
+        return ShellSetupState.Setup;
     }
 
     async setupScripts(): Promise<ShellScriptEditState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
+        const installations = await this.checkInstallations();
+
+        if (Array.from(installations.values()).every((installed) => !installed)) {
             return ShellScriptEditState.NotInstalled;
         }
 
-        try {
-            const profile = await getProfileForShell('pwsh');
-            const success = await setupPowerShellStartup('pwsh', profile);
-            return success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited;
-        } catch (err) {
-            traceError('Failed to setup PowerShell startup', err);
+        const anyEdited = [];
+        for (const [shell, installed] of installations.entries()) {
+            if (!installed) {
+                continue;
+            }
+
+            try {
+                const profile = await getProfileForShell(shell);
+                const success = await setupPowerShellStartup(shell, profile);
+                anyEdited.push(success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited);
+            } catch (err) {
+                traceError(`Failed to setup ${shell} startup`, err);
+            }
         }
-        return ShellScriptEditState.NotEdited;
+        return anyEdited.every((state) => state === ShellScriptEditState.Edited)
+            ? ShellScriptEditState.Edited
+            : ShellScriptEditState.NotEdited;
     }
 
     async teardownScripts(): Promise<ShellScriptEditState> {
-        const isInstalled = await this.checkInstallation();
-        if (!isInstalled) {
+        const installations = await this.checkInstallations();
+
+        if (Array.from(installations.values()).every((installed) => !installed)) {
             return ShellScriptEditState.NotInstalled;
         }
 
-        try {
-            const profile = await getProfileForShell('pwsh');
-            const success = await removePowerShellStartup('pwsh', profile);
-            return success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited;
-        } catch (err) {
-            traceError('Failed to remove PowerShell startup', err);
+        const anyEdited = [];
+        for (const [shell, installed] of installations.entries()) {
+            if (!installed) {
+                continue;
+            }
+
+            try {
+                const profile = await getProfileForShell(shell);
+                const success = await removePowerShellStartup(shell, profile);
+                anyEdited.push(success ? ShellScriptEditState.Edited : ShellScriptEditState.NotEdited);
+            } catch (err) {
+                traceError(`Failed to remove ${shell} startup`, err);
+            }
         }
-        return ShellScriptEditState.NotEdited;
+        return anyEdited.every((state) => state === ShellScriptEditState.Edited)
+            ? ShellScriptEditState.Edited
+            : ShellScriptEditState.NotEdited;
     }
+
     async clearCache(): Promise<void> {
-        await clearPwshCache('pwsh');
+        for (const shell of this._supportedShells) {
+            await clearPwshCache(shell);
+        }
+
+        // Reset installation check cache as well
+        this._isPwshInstalled = undefined;
+        this._isPs5Installed = undefined;
     }
 }