Skip to content

Commit 4050eda

Browse files
anthonykim1eleanorjboyd
authored andcommitted
Better handle terminal shell integration check (microsoft#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.
1 parent 5cd9b65 commit 4050eda

File tree

6 files changed

+34
-20
lines changed

6 files changed

+34
-20
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ 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+
if ((await shellIntegrationForActiveTerminal(name, profile)) && !isWsl()) {
7272
removeStartup(profile, key);
7373
return true;
7474
}

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { PythonCommandRunConfiguration, PythonEnvironment } from '../../../../api';
22
import { traceInfo } from '../../../../common/logging';
3+
import { sleep } from '../../../../common/utils/asyncUtils';
34
import { isWindows } from '../../../../common/utils/platformUtils';
45
import { activeTerminalShellIntegration } from '../../../../common/window.apis';
56
import { ShellConstants } from '../../../common/shellConstants';
67
import { quoteArgs } from '../../../execution/execUtils';
8+
import { SHELL_INTEGRATION_POLL_INTERVAL, SHELL_INTEGRATION_TIMEOUT } from '../../utils';
79

810
function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string {
911
const parts = [];
@@ -98,12 +100,19 @@ export function extractProfilePath(content: string): string | undefined {
98100
return undefined;
99101
}
100102

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

104113
if (hasShellIntegration) {
105114
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.`
115+
`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.`,
107116
);
108117
return true;
109118
}
@@ -112,8 +121,5 @@ export function shellIntegrationForActiveTerminal(name: string, profile?: string
112121

113122
export function isWsl(): boolean {
114123
// WSL sets these environment variables
115-
return !!(process.env.WSL_DISTRO_NAME ||
116-
process.env.WSL_INTEROP ||
117-
process.env.WSLENV);
124+
return !!(process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP || process.env.WSLENV);
118125
}
119-

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ 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+
if ((await shellIntegrationForActiveTerminal('fish', profilePath)) && !isWsl()) {
6262
removeFishStartup(profilePath, key);
6363
return true;
6464
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async function isPowerShellStartupSetup(shell: string, profile: string): Promise
146146
}
147147

148148
async function setupPowerShellStartup(shell: string, profile: string): Promise<boolean> {
149-
if (shellIntegrationForActiveTerminal(shell, profile) && !isWsl()) {
149+
if ((await shellIntegrationForActiveTerminal(shell, profile)) && !isWsl()) {
150150
removePowerShellStartup(shell, profile, POWERSHELL_OLD_ENV_KEY);
151151
removePowerShellStartup(shell, profile, POWERSHELL_ENV_KEY);
152152
return true;

src/features/terminal/terminalManager.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,10 @@ 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
}
@@ -145,26 +147,32 @@ export class TerminalManagerImpl implements TerminalManager {
145147
private async handleSetupCheck(shellType: string | Set<string>): Promise<void> {
146148
const shellTypes = typeof shellType === 'string' ? new Set([shellType]) : shellType;
147149
const providers = this.startupScriptProviders.filter((p) => shellTypes.has(p.shellType));
148-
if (providers.length > 0) {
150+
if (providers.length > 0) {
149151
const shellsToSetup: ShellStartupScriptProvider[] = [];
150152
await Promise.all(
151153
providers.map(async (p) => {
152154
const state = await p.isSetup();
153-
const currentSetup = (state === ShellSetupState.Setup);
155+
const currentSetup = state === ShellSetupState.Setup;
154156
// Check if we already processed this shell and the state hasn't changed
155157
if (this.shellSetup.has(p.shellType)) {
156158
const cachedSetup = this.shellSetup.get(p.shellType);
157159
if (currentSetup === cachedSetup) {
158160
traceVerbose(`Shell profile for ${p.shellType} already checked, state unchanged.`);
159161
return;
160162
}
161-
traceVerbose(`Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`);
163+
traceVerbose(
164+
`Shell profile for ${p.shellType} state changed from ${cachedSetup} to ${currentSetup}, re-evaluating.`,
165+
);
162166
}
163167
traceVerbose(`Checking shell profile for ${p.shellType}.`);
164168
if (state === ShellSetupState.NotSetup) {
165-
traceVerbose(`WSL detected: ${isWsl()}, Shell integration available: ${shellIntegrationForActiveTerminal(p.name)}`);
169+
traceVerbose(
170+
`WSL detected: ${isWsl()}, Shell integration available: ${await shellIntegrationForActiveTerminal(
171+
p.name,
172+
)}`,
173+
);
166174

167-
if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) {
175+
if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) {
168176
// Shell integration available and NOT in WSL - skip setup
169177
await p.teardownScripts();
170178
this.shellSetup.set(p.shellType, true);
@@ -180,7 +188,7 @@ export class TerminalManagerImpl implements TerminalManager {
180188
);
181189
}
182190
} else if (state === ShellSetupState.Setup) {
183-
if (shellIntegrationForActiveTerminal(p.name) && !isWsl()) {
191+
if ((await shellIntegrationForActiveTerminal(p.name)) && !isWsl()) {
184192
await p.teardownScripts();
185193
traceVerbose(
186194
`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)