Skip to content

Commit b341d79

Browse files
eleanorjboydanthonykim1
eleanorjboyd
and
anthonykim1
authored
cherry-pick shell changes to release (#923)
Resolves: #919 which will improve on top of #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. --------- Co-authored-by: Anthony Kim <62267334+anthonykim1@users.noreply.github.com>
1 parent 5cd9b65 commit b341d79

File tree

6 files changed

+97
-31
lines changed

6 files changed

+97
-31
lines changed

‎src/features/terminal/shells/bash/bashStartup.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import which from 'which';
55
import { traceError, traceInfo, traceVerbose } from '../../../../common/logging';
66
import { ShellConstants } from '../../../common/shellConstants';
77
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
8-
import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
8+
import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
99
import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
1010
import { BASH_ENV_KEY, BASH_OLD_ENV_KEY, BASH_SCRIPT_VERSION, ZSH_ENV_KEY, ZSH_OLD_ENV_KEY } from './bashConstants';
1111

@@ -68,7 +68,8 @@ async function isStartupSetup(profile: string, key: string): Promise<ShellSetupS
6868
return ShellSetupState.NotSetup;
6969
}
7070
async function setupStartup(profile: string, key: string, name: string): Promise<boolean> {
71-
if (shellIntegrationForActiveTerminal(name, profile) && !isWsl()) {
71+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
72+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(name, profile))) && !isWsl()) {
7273
removeStartup(profile, key);
7374
return true;
7475
}

‎src/features/terminal/shells/common/shellUtils.ts‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
import { PythonCommandRunConfiguration, PythonEnvironment } from '../../../../api';
22
import { traceInfo } from '../../../../common/logging';
3+
import { getGlobalPersistentState } from '../../../../common/persistentState';
4+
import { sleep } from '../../../../common/utils/asyncUtils';
35
import { isWindows } from '../../../../common/utils/platformUtils';
46
import { activeTerminalShellIntegration } from '../../../../common/window.apis';
7+
import { getConfiguration } from '../../../../common/workspace.apis';
58
import { ShellConstants } from '../../../common/shellConstants';
69
import { quoteArgs } from '../../../execution/execUtils';
10+
import { SHELL_INTEGRATION_POLL_INTERVAL, SHELL_INTEGRATION_TIMEOUT } from '../../utils';
11+
12+
export const SHELL_INTEGRATION_STATE_KEY = 'shellIntegration.enabled';
713

814
function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string {
915
const parts = [];
@@ -98,22 +104,60 @@ export function extractProfilePath(content: string): string | undefined {
98104
return undefined;
99105
}
100106

101-
export function shellIntegrationForActiveTerminal(name: string, profile?: string): boolean {
102-
const hasShellIntegration = activeTerminalShellIntegration();
107+
export async function shellIntegrationForActiveTerminal(name: string, profile?: string): Promise<boolean> {
108+
let hasShellIntegration = activeTerminalShellIntegration();
109+
let timeout = 0;
110+
111+
while (!hasShellIntegration && timeout < SHELL_INTEGRATION_TIMEOUT) {
112+
await sleep(SHELL_INTEGRATION_POLL_INTERVAL);
113+
timeout += SHELL_INTEGRATION_POLL_INTERVAL;
114+
hasShellIntegration = activeTerminalShellIntegration();
115+
}
103116

104117
if (hasShellIntegration) {
105118
traceInfo(
106-
`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.`
119+
`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.`,
107120
);
121+
122+
// Update persistent storage to reflect that shell integration is available
123+
const persistentState = await getGlobalPersistentState();
124+
await persistentState.set(SHELL_INTEGRATION_STATE_KEY, true);
125+
108126
return true;
109127
}
110128
return false;
111129
}
112130

113131
export function isWsl(): boolean {
114132
// WSL sets these environment variables
115-
return !!(process.env.WSL_DISTRO_NAME ||
116-
process.env.WSL_INTEROP ||
117-
process.env.WSLENV);
133+
return !!(process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP || process.env.WSLENV);
118134
}
119135

136+
export async function getShellIntegrationEnabledCache(): Promise<boolean> {
137+
const persistentState = await getGlobalPersistentState();
138+
const shellIntegrationInspect =
139+
getConfiguration('terminal.integrated').inspect<boolean>('shellIntegration.enabled');
140+
141+
let shellIntegrationEnabled = true;
142+
if (shellIntegrationInspect) {
143+
// Priority: workspaceFolder > workspace > globalRemoteValue > globalLocalValue > global > default
144+
const inspectValue = shellIntegrationInspect as Record<string, unknown>;
145+
146+
if (shellIntegrationInspect.workspaceFolderValue !== undefined) {
147+
shellIntegrationEnabled = shellIntegrationInspect.workspaceFolderValue;
148+
} else if (shellIntegrationInspect.workspaceValue !== undefined) {
149+
shellIntegrationEnabled = shellIntegrationInspect.workspaceValue;
150+
} else if ('globalRemoteValue' in shellIntegrationInspect && inspectValue.globalRemoteValue !== undefined) {
151+
shellIntegrationEnabled = inspectValue.globalRemoteValue as boolean;
152+
} else if ('globalLocalValue' in shellIntegrationInspect && inspectValue.globalLocalValue !== undefined) {
153+
shellIntegrationEnabled = inspectValue.globalLocalValue as boolean;
154+
} else if (shellIntegrationInspect.globalValue !== undefined) {
155+
shellIntegrationEnabled = shellIntegrationInspect.globalValue;
156+
} else if (shellIntegrationInspect.defaultValue !== undefined) {
157+
shellIntegrationEnabled = shellIntegrationInspect.defaultValue;
158+
}
159+
}
160+
161+
await persistentState.set(SHELL_INTEGRATION_STATE_KEY, shellIntegrationEnabled);
162+
return shellIntegrationEnabled;
163+
}

‎src/features/terminal/shells/fish/fishStartup.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import which from 'which';
66
import { traceError, traceInfo, traceVerbose } from '../../../../common/logging';
77
import { ShellConstants } from '../../../common/shellConstants';
88
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
9-
import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
9+
import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
1010
import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
1111
import { FISH_ENV_KEY, FISH_OLD_ENV_KEY, FISH_SCRIPT_VERSION } from './fishConstants';
1212

@@ -58,7 +58,8 @@ async function isStartupSetup(profilePath: string, key: string): Promise<boolean
5858

5959
async function setupStartup(profilePath: string, key: string): Promise<boolean> {
6060
try {
61-
if (shellIntegrationForActiveTerminal('fish', profilePath) && !isWsl()) {
61+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
62+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal('fish', profilePath))) && !isWsl()) {
6263
removeFishStartup(profilePath, key);
6364
return true;
6465
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { ShellConstants } from '../../../common/shellConstants';
1313
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
1414
import {
1515
extractProfilePath,
16+
getShellIntegrationEnabledCache,
1617
isWsl,
1718
PROFILE_TAG_END,
1819
PROFILE_TAG_START,
@@ -146,7 +147,9 @@ async function isPowerShellStartupSetup(shell: string, profile: string): Promise
146147
}
147148

148149
async function setupPowerShellStartup(shell: string, profile: string): Promise<boolean> {
149-
if (shellIntegrationForActiveTerminal(shell, profile) && !isWsl()) {
150+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
151+
152+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(shell, profile))) && !isWsl()) {
150153
removePowerShellStartup(shell, profile, POWERSHELL_OLD_ENV_KEY);
151154
removePowerShellStartup(shell, profile, POWERSHELL_ENV_KEY);
152155
return true;

‎src/features/terminal/terminalManager.ts‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { getConfiguration, onDidChangeConfiguration } from '../../common/workspa
1616
import { isActivatableEnvironment } from '../common/activation';
1717
import { identifyTerminalShell } from '../common/shellDetector';
1818
import { getPythonApi } from '../pythonApi';
19-
import { isWsl, shellIntegrationForActiveTerminal } from './shells/common/shellUtils';
19+
import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from './shells/common/shellUtils';
2020
import { ShellEnvsProvider, ShellSetupState, ShellStartupScriptProvider } from './shells/startupProvider';
2121
import { handleSettingUpShellProfile } from './shellStartupSetupHandlers';
2222
import {
@@ -129,12 +129,28 @@ export class TerminalManagerImpl implements TerminalManager {
129129
await this.handleSetupCheck(shells);
130130
}
131131
} else {
132-
traceVerbose(`Auto activation type changed to ${actType}, we are cleaning up shell startup setup`);
133-
// Teardown scripts when switching away from shell startup activation
132+
traceVerbose(
133+
`Auto activation type changed to ${actType}, we are cleaning up shell startup setup`,
134+
);
135+
// Teardown scripts when switching away from shell startup activation
134136
await Promise.all(this.startupScriptProviders.map((p) => p.teardownScripts()));
135137
this.shellSetup.clear();
136138
}
137139
}
140+
if (e.affectsConfiguration('terminal.integrated.shellIntegration.enabled')) {
141+
traceInfo('Shell integration setting changed, invalidating cache');
142+
const updatedShellIntegrationSetting = await getShellIntegrationEnabledCache();
143+
if (!updatedShellIntegrationSetting) {
144+
const shells = new Set(
145+
terminals()
146+
.map((t) => identifyTerminalShell(t))
147+
.filter((t) => t !== 'unknown'),
148+
);
149+
if (shells.size > 0) {
150+
await this.handleSetupCheck(shells);
151+
}
152+
}
153+
}
138154
}),
139155
onDidChangeWindowState((e) => {
140156
this.hasFocus = e.focused;
@@ -145,26 +161,24 @@ export class TerminalManagerImpl implements TerminalManager {
145161
private async handleSetupCheck(shellType: string | Set<string>): Promise<void> {
146162
const shellTypes = typeof shellType === 'string' ? new Set([shellType]) : shellType;
147163
const providers = this.startupScriptProviders.filter((p) => shellTypes.has(p.shellType));
148-
if (providers.length > 0) {
164+
if (providers.length > 0) {
149165
const shellsToSetup: ShellStartupScriptProvider[] = [];
150166
await Promise.all(
151167
providers.map(async (p) => {
152168
const state = await p.isSetup();
153-
const currentSetup = (state === ShellSetupState.Setup);
154-
// Check if we already processed this shell and the state hasn't changed
155-
if (this.shellSetup.has(p.shellType)) {
156-
const cachedSetup = this.shellSetup.get(p.shellType);
157-
if (currentSetup === cachedSetup) {
158-
traceVerbose(`Shell profile for ${p.shellType} already checked, state unchanged.`);
159-
return;
160-
}
161-
traceVerbose(`Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`);
162-
}
163-
traceVerbose(`Checking shell profile for ${p.shellType}.`);
169+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
170+
traceVerbose(`Checking shell profile for ${p.shellType}, with state: ${state}`);
164171
if (state === ShellSetupState.NotSetup) {
165-
traceVerbose(`WSL detected: ${isWsl()}, Shell integration available: ${shellIntegrationForActiveTerminal(p.name)}`);
172+
traceVerbose(
173+
`WSL detected: ${isWsl()}, Shell integration available from setting, or active terminal: ${shellIntegrationEnabled}, or ${await shellIntegrationForActiveTerminal(
174+
p.name,
175+
)}`,
176+
);
166177

167-
if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) {
178+
if (
179+
(shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(p.name))) &&
180+
!isWsl()
181+
) {
168182
// Shell integration available and NOT in WSL - skip setup
169183
await p.teardownScripts();
170184
this.shellSetup.set(p.shellType, true);
@@ -180,7 +194,10 @@ export class TerminalManagerImpl implements TerminalManager {
180194
);
181195
}
182196
} else if (state === ShellSetupState.Setup) {
183-
if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) {
197+
if (
198+
(shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(p.name))) &&
199+
!isWsl()
200+
) {
184201
await p.teardownScripts();
185202
traceVerbose(
186203
`Shell integration available for ${p.shellType}, removed profile script in favor of shell integration.`,

‎src/features/terminal/utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import { PythonEnvironment, PythonProject, PythonProjectEnvironmentApi, PythonPr
44
import { sleep } from '../../common/utils/asyncUtils';
55
import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis';
66

7-
const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds
8-
const SHELL_INTEGRATION_POLL_INTERVAL = 20; // 0.02 seconds
7+
export const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds
8+
export const SHELL_INTEGRATION_POLL_INTERVAL = 20; // 0.02 seconds
99

1010
export async function waitForShellIntegration(terminal: Terminal): Promise<boolean> {
1111
let timeout = 0;

0 commit comments

Comments
 (0)