From 4050edaa325bd0f925e7fc0f7128c1c460dc96f9 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Wed, 8 Oct 2025 11:24:29 -0700 Subject: [PATCH 1/2] Better handle terminal shell integration check (#915) This would incur less notification for change in user profile rather try to wait for shell integration not too long(max 0.5 second, but more commonly earlier since shell integration would be ready before then), we were already taking similar approach in some part of the code, but not all. --- .../terminal/shells/bash/bashStartup.ts | 2 +- .../terminal/shells/common/shellUtils.ts | 20 ++++++++++------ .../terminal/shells/fish/fishStartup.ts | 2 +- .../terminal/shells/pwsh/pwshStartup.ts | 2 +- src/features/terminal/terminalManager.ts | 24 ++++++++++++------- src/features/terminal/utils.ts | 4 ++-- 6 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/features/terminal/shells/bash/bashStartup.ts b/src/features/terminal/shells/bash/bashStartup.ts index bbe98c85..69643a7e 100644 --- a/src/features/terminal/shells/bash/bashStartup.ts +++ b/src/features/terminal/shells/bash/bashStartup.ts @@ -68,7 +68,7 @@ async function isStartupSetup(profile: string, key: string): Promise { - if (shellIntegrationForActiveTerminal(name, profile) && !isWsl()) { + if ((await shellIntegrationForActiveTerminal(name, profile)) && !isWsl()) { removeStartup(profile, key); return true; } diff --git a/src/features/terminal/shells/common/shellUtils.ts b/src/features/terminal/shells/common/shellUtils.ts index 20d310b5..5dae76a9 100644 --- a/src/features/terminal/shells/common/shellUtils.ts +++ b/src/features/terminal/shells/common/shellUtils.ts @@ -1,9 +1,11 @@ import { PythonCommandRunConfiguration, PythonEnvironment } from '../../../../api'; import { traceInfo } from '../../../../common/logging'; +import { sleep } from '../../../../common/utils/asyncUtils'; import { isWindows } from '../../../../common/utils/platformUtils'; import { activeTerminalShellIntegration } from '../../../../common/window.apis'; import { ShellConstants } from '../../../common/shellConstants'; import { quoteArgs } from '../../../execution/execUtils'; +import { SHELL_INTEGRATION_POLL_INTERVAL, SHELL_INTEGRATION_TIMEOUT } from '../../utils'; function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string { const parts = []; @@ -98,12 +100,19 @@ export function extractProfilePath(content: string): string | undefined { return undefined; } -export function shellIntegrationForActiveTerminal(name: string, profile?: string): boolean { - const hasShellIntegration = activeTerminalShellIntegration(); +export async function shellIntegrationForActiveTerminal(name: string, profile?: string): Promise { + let hasShellIntegration = activeTerminalShellIntegration(); + let timeout = 0; + + while (!hasShellIntegration && timeout < SHELL_INTEGRATION_TIMEOUT) { + await sleep(SHELL_INTEGRATION_POLL_INTERVAL); + timeout += SHELL_INTEGRATION_POLL_INTERVAL; + hasShellIntegration = activeTerminalShellIntegration(); + } if (hasShellIntegration) { traceInfo( - `SHELL: Shell integration is available on your active terminal, with name ${name} and profile ${profile}. Python activate scripts will be evaluated at shell integration level, except in WSL.` + `SHELL: Shell integration is available on your active terminal, with name ${name} and profile ${profile}. Python activate scripts will be evaluated at shell integration level, except in WSL.`, ); return true; } @@ -112,8 +121,5 @@ export function shellIntegrationForActiveTerminal(name: string, profile?: string export function isWsl(): boolean { // WSL sets these environment variables - return !!(process.env.WSL_DISTRO_NAME || - process.env.WSL_INTEROP || - process.env.WSLENV); + return !!(process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP || process.env.WSLENV); } - diff --git a/src/features/terminal/shells/fish/fishStartup.ts b/src/features/terminal/shells/fish/fishStartup.ts index 395829e9..6621e141 100644 --- a/src/features/terminal/shells/fish/fishStartup.ts +++ b/src/features/terminal/shells/fish/fishStartup.ts @@ -58,7 +58,7 @@ async function isStartupSetup(profilePath: string, key: string): Promise { try { - if (shellIntegrationForActiveTerminal('fish', profilePath) && !isWsl()) { + if ((await shellIntegrationForActiveTerminal('fish', profilePath)) && !isWsl()) { removeFishStartup(profilePath, key); return true; } diff --git a/src/features/terminal/shells/pwsh/pwshStartup.ts b/src/features/terminal/shells/pwsh/pwshStartup.ts index 3758dbb7..7c94dffe 100644 --- a/src/features/terminal/shells/pwsh/pwshStartup.ts +++ b/src/features/terminal/shells/pwsh/pwshStartup.ts @@ -146,7 +146,7 @@ async function isPowerShellStartupSetup(shell: string, profile: string): Promise } async function setupPowerShellStartup(shell: string, profile: string): Promise { - if (shellIntegrationForActiveTerminal(shell, profile) && !isWsl()) { + if ((await shellIntegrationForActiveTerminal(shell, profile)) && !isWsl()) { removePowerShellStartup(shell, profile, POWERSHELL_OLD_ENV_KEY); removePowerShellStartup(shell, profile, POWERSHELL_ENV_KEY); return true; diff --git a/src/features/terminal/terminalManager.ts b/src/features/terminal/terminalManager.ts index 81768dbd..b66f95e6 100644 --- a/src/features/terminal/terminalManager.ts +++ b/src/features/terminal/terminalManager.ts @@ -129,8 +129,10 @@ export class TerminalManagerImpl implements TerminalManager { await this.handleSetupCheck(shells); } } else { - traceVerbose(`Auto activation type changed to ${actType}, we are cleaning up shell startup setup`); - // Teardown scripts when switching away from shell startup activation + traceVerbose( + `Auto activation type changed to ${actType}, we are cleaning up shell startup setup`, + ); + // Teardown scripts when switching away from shell startup activation await Promise.all(this.startupScriptProviders.map((p) => p.teardownScripts())); this.shellSetup.clear(); } @@ -145,12 +147,12 @@ export class TerminalManagerImpl implements TerminalManager { private async handleSetupCheck(shellType: string | Set): Promise { const shellTypes = typeof shellType === 'string' ? new Set([shellType]) : shellType; const providers = this.startupScriptProviders.filter((p) => shellTypes.has(p.shellType)); - if (providers.length > 0) { + if (providers.length > 0) { const shellsToSetup: ShellStartupScriptProvider[] = []; await Promise.all( providers.map(async (p) => { const state = await p.isSetup(); - const currentSetup = (state === ShellSetupState.Setup); + const currentSetup = state === ShellSetupState.Setup; // Check if we already processed this shell and the state hasn't changed if (this.shellSetup.has(p.shellType)) { const cachedSetup = this.shellSetup.get(p.shellType); @@ -158,13 +160,19 @@ export class TerminalManagerImpl implements TerminalManager { traceVerbose(`Shell profile for ${p.shellType} already checked, state unchanged.`); return; } - traceVerbose(`Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`); + traceVerbose( + `Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`, + ); } traceVerbose(`Checking shell profile for ${p.shellType}.`); if (state === ShellSetupState.NotSetup) { - traceVerbose(`WSL detected: ${isWsl()}, Shell integration available: ${shellIntegrationForActiveTerminal(p.name)}`); + traceVerbose( + `WSL detected: ${isWsl()}, Shell integration available: ${await shellIntegrationForActiveTerminal( + p.name, + )}`, + ); - if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) { + if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) { // Shell integration available and NOT in WSL - skip setup await p.teardownScripts(); this.shellSetup.set(p.shellType, true); @@ -180,7 +188,7 @@ export class TerminalManagerImpl implements TerminalManager { ); } } else if (state === ShellSetupState.Setup) { - if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) { + if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) { await p.teardownScripts(); traceVerbose( `Shell integration available for ${p.shellType}, removed profile script in favor of shell integration.`, diff --git a/src/features/terminal/utils.ts b/src/features/terminal/utils.ts index 9476ac10..c8f6443d 100644 --- a/src/features/terminal/utils.ts +++ b/src/features/terminal/utils.ts @@ -4,8 +4,8 @@ import { PythonEnvironment, PythonProject, PythonProjectEnvironmentApi, PythonPr import { sleep } from '../../common/utils/asyncUtils'; import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis'; -const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds -const SHELL_INTEGRATION_POLL_INTERVAL = 20; // 0.02 seconds +export const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds +export const SHELL_INTEGRATION_POLL_INTERVAL = 20; // 0.02 seconds export async function waitForShellIntegration(terminal: Terminal): Promise { let timeout = 0; From fac64a131399489affd1f1e8187ceb633c21954d Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 9 Oct 2025 00:20:44 -0700 Subject: [PATCH 2/2] Improve shell startup experience using setting value (#921) Resolves: https://github.com/microsoft/vscode-python-environments/issues/919 which will improve on top of https://github.com/microsoft/vscode-python-environments/pull/915 I want to bulletproof shell startup as much as possible. We shouldn't be showing profile modification prompt if user has shell integration. We should also ensure proper clean up. --- .../terminal/shells/bash/bashStartup.ts | 5 ++- .../terminal/shells/common/shellUtils.ts | 38 ++++++++++++++++ .../terminal/shells/fish/fishStartup.ts | 5 ++- .../terminal/shells/pwsh/pwshStartup.ts | 5 ++- src/features/terminal/terminalManager.ts | 43 +++++++++++-------- 5 files changed, 74 insertions(+), 22 deletions(-) diff --git a/src/features/terminal/shells/bash/bashStartup.ts b/src/features/terminal/shells/bash/bashStartup.ts index 69643a7e..79292169 100644 --- a/src/features/terminal/shells/bash/bashStartup.ts +++ b/src/features/terminal/shells/bash/bashStartup.ts @@ -5,7 +5,7 @@ import which from 'which'; import { traceError, traceInfo, traceVerbose } from '../../../../common/logging'; import { ShellConstants } from '../../../common/shellConstants'; import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils'; -import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils'; +import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils'; import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider'; import { BASH_ENV_KEY, BASH_OLD_ENV_KEY, BASH_SCRIPT_VERSION, ZSH_ENV_KEY, ZSH_OLD_ENV_KEY } from './bashConstants'; @@ -68,7 +68,8 @@ async function isStartupSetup(profile: string, key: string): Promise { - if ((await shellIntegrationForActiveTerminal(name, profile)) && !isWsl()) { + const shellIntegrationEnabled = await getShellIntegrationEnabledCache(); + if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(name, profile))) && !isWsl()) { removeStartup(profile, key); return true; } diff --git a/src/features/terminal/shells/common/shellUtils.ts b/src/features/terminal/shells/common/shellUtils.ts index 5dae76a9..381be880 100644 --- a/src/features/terminal/shells/common/shellUtils.ts +++ b/src/features/terminal/shells/common/shellUtils.ts @@ -1,12 +1,16 @@ import { PythonCommandRunConfiguration, PythonEnvironment } from '../../../../api'; import { traceInfo } from '../../../../common/logging'; +import { getGlobalPersistentState } from '../../../../common/persistentState'; import { sleep } from '../../../../common/utils/asyncUtils'; import { isWindows } from '../../../../common/utils/platformUtils'; import { activeTerminalShellIntegration } from '../../../../common/window.apis'; +import { getConfiguration } from '../../../../common/workspace.apis'; import { ShellConstants } from '../../../common/shellConstants'; import { quoteArgs } from '../../../execution/execUtils'; import { SHELL_INTEGRATION_POLL_INTERVAL, SHELL_INTEGRATION_TIMEOUT } from '../../utils'; +export const SHELL_INTEGRATION_STATE_KEY = 'shellIntegration.enabled'; + function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string { const parts = []; for (const cmd of command) { @@ -114,6 +118,11 @@ export async function shellIntegrationForActiveTerminal(name: string, profile?: traceInfo( `SHELL: Shell integration is available on your active terminal, with name ${name} and profile ${profile}. Python activate scripts will be evaluated at shell integration level, except in WSL.`, ); + + // Update persistent storage to reflect that shell integration is available + const persistentState = await getGlobalPersistentState(); + await persistentState.set(SHELL_INTEGRATION_STATE_KEY, true); + return true; } return false; @@ -123,3 +132,32 @@ export function isWsl(): boolean { // WSL sets these environment variables return !!(process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP || process.env.WSLENV); } + +export async function getShellIntegrationEnabledCache(): Promise { + const persistentState = await getGlobalPersistentState(); + const shellIntegrationInspect = + getConfiguration('terminal.integrated').inspect('shellIntegration.enabled'); + + let shellIntegrationEnabled = true; + if (shellIntegrationInspect) { + // Priority: workspaceFolder > workspace > globalRemoteValue > globalLocalValue > global > default + const inspectValue = shellIntegrationInspect as Record; + + if (shellIntegrationInspect.workspaceFolderValue !== undefined) { + shellIntegrationEnabled = shellIntegrationInspect.workspaceFolderValue; + } else if (shellIntegrationInspect.workspaceValue !== undefined) { + shellIntegrationEnabled = shellIntegrationInspect.workspaceValue; + } else if ('globalRemoteValue' in shellIntegrationInspect && inspectValue.globalRemoteValue !== undefined) { + shellIntegrationEnabled = inspectValue.globalRemoteValue as boolean; + } else if ('globalLocalValue' in shellIntegrationInspect && inspectValue.globalLocalValue !== undefined) { + shellIntegrationEnabled = inspectValue.globalLocalValue as boolean; + } else if (shellIntegrationInspect.globalValue !== undefined) { + shellIntegrationEnabled = shellIntegrationInspect.globalValue; + } else if (shellIntegrationInspect.defaultValue !== undefined) { + shellIntegrationEnabled = shellIntegrationInspect.defaultValue; + } + } + + await persistentState.set(SHELL_INTEGRATION_STATE_KEY, shellIntegrationEnabled); + return shellIntegrationEnabled; +} diff --git a/src/features/terminal/shells/fish/fishStartup.ts b/src/features/terminal/shells/fish/fishStartup.ts index 6621e141..40de3e98 100644 --- a/src/features/terminal/shells/fish/fishStartup.ts +++ b/src/features/terminal/shells/fish/fishStartup.ts @@ -6,7 +6,7 @@ import which from 'which'; import { traceError, traceInfo, traceVerbose } from '../../../../common/logging'; import { ShellConstants } from '../../../common/shellConstants'; import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils'; -import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils'; +import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils'; import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider'; import { FISH_ENV_KEY, FISH_OLD_ENV_KEY, FISH_SCRIPT_VERSION } from './fishConstants'; @@ -58,7 +58,8 @@ async function isStartupSetup(profilePath: string, key: string): Promise { try { - if ((await shellIntegrationForActiveTerminal('fish', profilePath)) && !isWsl()) { + const shellIntegrationEnabled = await getShellIntegrationEnabledCache(); + if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal('fish', profilePath))) && !isWsl()) { removeFishStartup(profilePath, key); return true; } diff --git a/src/features/terminal/shells/pwsh/pwshStartup.ts b/src/features/terminal/shells/pwsh/pwshStartup.ts index 7c94dffe..6a3b4dee 100644 --- a/src/features/terminal/shells/pwsh/pwshStartup.ts +++ b/src/features/terminal/shells/pwsh/pwshStartup.ts @@ -13,6 +13,7 @@ import { ShellConstants } from '../../../common/shellConstants'; import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils'; import { extractProfilePath, + getShellIntegrationEnabledCache, isWsl, PROFILE_TAG_END, PROFILE_TAG_START, @@ -146,7 +147,9 @@ async function isPowerShellStartupSetup(shell: string, profile: string): Promise } async function setupPowerShellStartup(shell: string, profile: string): Promise { - if ((await shellIntegrationForActiveTerminal(shell, profile)) && !isWsl()) { + const shellIntegrationEnabled = await getShellIntegrationEnabledCache(); + + if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(shell, profile))) && !isWsl()) { removePowerShellStartup(shell, profile, POWERSHELL_OLD_ENV_KEY); removePowerShellStartup(shell, profile, POWERSHELL_ENV_KEY); return true; diff --git a/src/features/terminal/terminalManager.ts b/src/features/terminal/terminalManager.ts index b66f95e6..f7acb2d4 100644 --- a/src/features/terminal/terminalManager.ts +++ b/src/features/terminal/terminalManager.ts @@ -16,7 +16,7 @@ import { getConfiguration, onDidChangeConfiguration } from '../../common/workspa import { isActivatableEnvironment } from '../common/activation'; import { identifyTerminalShell } from '../common/shellDetector'; import { getPythonApi } from '../pythonApi'; -import { isWsl, shellIntegrationForActiveTerminal } from './shells/common/shellUtils'; +import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from './shells/common/shellUtils'; import { ShellEnvsProvider, ShellSetupState, ShellStartupScriptProvider } from './shells/startupProvider'; import { handleSettingUpShellProfile } from './shellStartupSetupHandlers'; import { @@ -137,6 +137,20 @@ export class TerminalManagerImpl implements TerminalManager { this.shellSetup.clear(); } } + if (e.affectsConfiguration('terminal.integrated.shellIntegration.enabled')) { + traceInfo('Shell integration setting changed, invalidating cache'); + const updatedShellIntegrationSetting = await getShellIntegrationEnabledCache(); + if (!updatedShellIntegrationSetting) { + const shells = new Set( + terminals() + .map((t) => identifyTerminalShell(t)) + .filter((t) => t !== 'unknown'), + ); + if (shells.size > 0) { + await this.handleSetupCheck(shells); + } + } + } }), onDidChangeWindowState((e) => { this.hasFocus = e.focused; @@ -152,27 +166,19 @@ export class TerminalManagerImpl implements TerminalManager { await Promise.all( providers.map(async (p) => { const state = await p.isSetup(); - const currentSetup = state === ShellSetupState.Setup; - // Check if we already processed this shell and the state hasn't changed - if (this.shellSetup.has(p.shellType)) { - const cachedSetup = this.shellSetup.get(p.shellType); - if (currentSetup === cachedSetup) { - traceVerbose(`Shell profile for ${p.shellType} already checked, state unchanged.`); - return; - } - traceVerbose( - `Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`, - ); - } - traceVerbose(`Checking shell profile for ${p.shellType}.`); + const shellIntegrationEnabled = await getShellIntegrationEnabledCache(); + traceVerbose(`Checking shell profile for ${p.shellType}, with state: ${state}`); if (state === ShellSetupState.NotSetup) { traceVerbose( - `WSL detected: ${isWsl()}, Shell integration available: ${await shellIntegrationForActiveTerminal( + `WSL detected: ${isWsl()}, Shell integration available from setting, or active terminal: ${shellIntegrationEnabled}, or ${await shellIntegrationForActiveTerminal( p.name, )}`, ); - if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) { + if ( + (shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(p.name))) && + !isWsl() + ) { // Shell integration available and NOT in WSL - skip setup await p.teardownScripts(); this.shellSetup.set(p.shellType, true); @@ -188,7 +194,10 @@ export class TerminalManagerImpl implements TerminalManager { ); } } else if (state === ShellSetupState.Setup) { - if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) { + if ( + (shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(p.name))) && + !isWsl() + ) { await p.teardownScripts(); traceVerbose( `Shell integration available for ${p.shellType}, removed profile script in favor of shell integration.`,