Skip to content

Commit f95281b

Browse files
karthiknadigkarthiknadig
committed
fix: improve shell setup handling
1 parent d902fb0 commit f95281b

File tree

3 files changed

+65
-53
lines changed

3 files changed

+65
-53
lines changed

‎src/features/terminal/shellStartupSetupHandlers.ts‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,14 @@ import { l10n } from 'vscode';
22
import { executeCommand } from '../../common/command.api';
33
import { Common, ShellStartupActivationStrings } from '../../common/localize';
44
import { traceInfo, traceVerbose } from '../../common/logging';
5-
import { isWindows } from '../../common/utils/platformUtils';
65
import { showErrorMessage, showInformationMessage } from '../../common/window.apis';
7-
import { ShellConstants } from '../common/shellConstants';
86
import { ShellScriptEditState, ShellStartupScriptProvider } from './shells/startupProvider';
97
import { getAutoActivationType, setAutoActivationType } from './utils';
108

11-
function getProviders(shellTypes: Set<string>, providers: ShellStartupScriptProvider[]): ShellStartupScriptProvider[] {
12-
return providers.filter((provider) => {
13-
if (isWindows() && shellTypes.has(ShellConstants.PWSH) && provider.shellType === 'powershell') {
14-
return true;
15-
}
16-
return shellTypes.has(provider.shellType);
17-
});
18-
}
19-
209
export async function handleSettingUpShellProfile(
21-
shellTypes: Set<string>,
22-
allProviders: ShellStartupScriptProvider[],
10+
providers: ShellStartupScriptProvider[],
2311
callback: (provider: ShellStartupScriptProvider, result: boolean) => void,
2412
): Promise<void> {
25-
const providers = getProviders(shellTypes, allProviders);
26-
if (providers.length === 0) {
27-
traceVerbose('No shell providers found for the specified shell types: ' + Array.from(shellTypes).join(', '));
28-
}
2913
const shells = providers.map((p) => p.shellType).join(', ');
3014
const response = await showInformationMessage(
3115
l10n.t(
@@ -39,7 +23,9 @@ export async function handleSettingUpShellProfile(
3923

4024
if (response === Common.yes) {
4125
traceVerbose(`User chose to set up shell profiles for ${shells} shells`);
42-
const states = await Promise.all(providers.map((provider) => provider.setupScripts()));
26+
const states = (await Promise.all(providers.map((provider) => provider.setupScripts()))).filter(
27+
(state) => state !== ShellScriptEditState.NotInstalled,
28+
);
4329
if (states.every((state) => state === ShellScriptEditState.Edited)) {
4430
setImmediate(async () => {
4531
await showInformationMessage(

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ const regionEnd = '#endregion vscode python';
117117
function getActivationContent(): string {
118118
const lineSep = isWindows() ? '\r\n' : '\n';
119119
const activationContent = [
120-
`# version: ${PWSH_SCRIPT_VERSION}`,
120+
`#version: ${PWSH_SCRIPT_VERSION}`,
121121
`if (($env:TERM_PROGRAM -eq 'vscode') -and ($null -ne $env:${POWERSHELL_ENV_KEY})) {`,
122122
' try {',
123123
` Invoke-Expression $env:${POWERSHELL_ENV_KEY}`,
@@ -199,7 +199,6 @@ export class PowerShellClassicStartupProvider implements ShellStartupScriptProvi
199199
async isSetup(): Promise<ShellSetupState> {
200200
const isInstalled = await this.checkInstallation();
201201
if (!isInstalled) {
202-
traceVerbose('PowerShell is not installed');
203202
return ShellSetupState.NotInstalled;
204203
}
205204

@@ -232,7 +231,6 @@ export class PowerShellClassicStartupProvider implements ShellStartupScriptProvi
232231
async teardownScripts(): Promise<ShellScriptEditState> {
233232
const isInstalled = await this.checkInstallation();
234233
if (!isInstalled) {
235-
traceVerbose('PowerShell is not installed');
236234
return ShellScriptEditState.NotInstalled;
237235
}
238236

@@ -282,7 +280,6 @@ export class PwshStartupProvider implements ShellStartupScriptProvider {
282280
async setupScripts(): Promise<ShellScriptEditState> {
283281
const isInstalled = await this.checkInstallation();
284282
if (!isInstalled) {
285-
traceVerbose('PowerShell is not installed');
286283
return ShellScriptEditState.NotInstalled;
287284
}
288285

@@ -299,7 +296,6 @@ export class PwshStartupProvider implements ShellStartupScriptProvider {
299296
async teardownScripts(): Promise<ShellScriptEditState> {
300297
const isInstalled = await this.checkInstallation();
301298
if (!isInstalled) {
302-
traceVerbose('PowerShell is not installed');
303299
return ShellScriptEditState.NotInstalled;
304300
}
305301

‎src/features/terminal/terminalManager.ts‎

Lines changed: 60 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { getConfiguration, onDidChangeConfiguration } from '../../common/workspa
1414
import { isActivatableEnvironment } from '../common/activation';
1515
import { identifyTerminalShell } from '../common/shellDetector';
1616
import { getPythonApi } from '../pythonApi';
17-
import { ShellEnvsProvider, ShellStartupScriptProvider } from './shells/startupProvider';
18-
import { handleSettingUpShellProfile } from './shellStartupSetupHandlers';
17+
import { ShellEnvsProvider, ShellSetupState, ShellStartupScriptProvider } from './shells/startupProvider';
18+
import { filterProviders, handleSettingUpShellProfile } from './shellStartupSetupHandlers';
1919
import {
2020
DidChangeTerminalActivationStateEvent,
2121
TerminalActivation,
@@ -113,45 +113,77 @@ export class TerminalManagerImpl implements TerminalManager {
113113
.filter((t) => t !== 'unknown'),
114114
);
115115
if (shells.size > 0) {
116-
await handleSettingUpShellProfile(shells, this.startupScriptProviders, (p, v) =>
117-
this.shellSetup.set(p.shellType, v),
118-
);
116+
await this.handleSetupCheck(shells);
119117
}
120118
}
121119
}
122120
}),
123121
);
124122
}
125123

126-
private async getEffectiveActivationType(shellType: string): Promise<AutoActivationType> {
127-
const provider = this.startupScriptProviders.find((p) => p.shellType === shellType);
128-
if (provider) {
129-
traceVerbose(`Shell startup is supported for ${shellType}, using shell startup activation`);
130-
const isSetup = this.shellSetup.get(shellType);
131-
if (isSetup === true) {
132-
traceVerbose(`Shell profile for ${shellType} is already setup.`);
133-
return 'shellStartup';
134-
} else if (isSetup === false) {
135-
traceVerbose(`Shell profile for ${shellType} is not set up, using command fallback.`);
136-
return 'command';
137-
}
124+
private async handleSetupCheck(shellType: string | Set<string>): Promise<void> {
125+
const shellTypes = typeof shellType === 'string' ? new Set([shellType]) : shellType;
126+
const providers = filterProviders(shellTypes, this.startupScriptProviders);
127+
if (providers.length > 0) {
128+
const shellsToSetup: ShellStartupScriptProvider[] = [];
129+
await Promise.all(
130+
providers.map(async (p) => {
131+
if (this.shellSetup.has(p.shellType)) {
132+
traceVerbose(`Shell profile for ${p.shellType} already checked.`);
133+
return;
134+
}
135+
traceVerbose(`Checking shell profile for ${p.shellType}.`);
136+
const state = await p.isSetup();
137+
if (state === ShellSetupState.NotSetup) {
138+
this.shellSetup.set(p.shellType, false);
139+
shellsToSetup.push(p);
140+
traceVerbose(`Shell profile for ${p.shellType} is not setup.`);
141+
} else if (state === ShellSetupState.Setup) {
142+
this.shellSetup.set(p.shellType, true);
143+
traceVerbose(`Shell profile for ${p.shellType} is setup.`);
144+
} else if (state === ShellSetupState.NotInstalled) {
145+
this.shellSetup.set(p.shellType, false);
146+
traceVerbose(`Shell profile for ${p.shellType} is not installed.`);
147+
}
148+
}),
149+
);
138150

139-
if (await provider.isSetup()) {
140-
this.shellSetup.set(shellType, true);
141-
traceVerbose(`Shell profile for ${shellType} is setup successfully.`);
142-
return 'shellStartup';
151+
if (shellsToSetup.length === 0) {
152+
traceVerbose(`No shell profiles to setup for ${Array.from(shellTypes).join(', ')}`);
153+
return;
143154
}
144155

145-
this.shellSetup.set(shellType, false);
146-
traceVerbose(`Shell profile for ${shellType} is not setup, falling back to command activation`);
147-
148156
setImmediate(async () => {
149-
await handleSettingUpShellProfile(new Set([shellType]), this.startupScriptProviders, (p, v) =>
150-
this.shellSetup.set(p.shellType, v),
151-
);
157+
await handleSettingUpShellProfile(shellsToSetup, (p, v) => this.shellSetup.set(p.shellType, v));
152158
});
159+
}
160+
}
161+
162+
private getShellActivationType(shellType: string): AutoActivationType | undefined {
163+
let isSetup = this.shellSetup.get(shellType);
164+
if (isSetup === true) {
165+
traceVerbose(`Shell profile for ${shellType} is already setup.`);
166+
return 'shellStartup';
167+
} else if (isSetup === false) {
168+
traceVerbose(`Shell profile for ${shellType} is not set up, using command fallback.`);
153169
return 'command';
154170
}
171+
}
172+
173+
private async getEffectiveActivationType(shellType: string): Promise<AutoActivationType> {
174+
const providers = filterProviders(new Set([shellType]), this.startupScriptProviders);
175+
if (providers.length > 0) {
176+
traceVerbose(`Shell startup is supported for ${shellType}, using shell startup activation`);
177+
let isSetup = this.getShellActivationType(shellType);
178+
if (isSetup !== undefined) {
179+
return isSetup;
180+
}
181+
182+
await this.handleSetupCheck(shellType);
183+
184+
// Check again after the setup check.
185+
return this.getShellActivationType(shellType) ?? 'command';
186+
}
155187
traceInfo(`Shell startup not supported for ${shellType}, using command activation as fallback`);
156188
return 'command';
157189
}
@@ -322,9 +354,7 @@ export class TerminalManagerImpl implements TerminalManager {
322354
.filter((t) => t !== 'unknown'),
323355
);
324356
if (shells.size > 0) {
325-
await handleSettingUpShellProfile(shells, this.startupScriptProviders, (p, v) =>
326-
this.shellSetup.set(p.shellType, v),
327-
);
357+
await this.handleSetupCheck(shells);
328358
}
329359
}
330360
}

0 commit comments

Comments
 (0)