diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index a450c5b02162..ae591e1d7be8 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -130,7 +130,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.disposables, ); const { shell } = this.applicationEnvironment; - const isActive = await this.shellIntegrationService.isWorking(shell); + const isActive = await this.shellIntegrationService.isWorking(); const shellType = identifyShellFromShellPath(shell); if (!isActive && shellType !== TerminalShellType.commandPrompt) { traceWarn( @@ -218,6 +218,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } if (key === 'PATH') { + const options = { + applyAtShellIntegration: true, + applyAtProcessCreation: true, + }; if (processEnv.PATH && env.PATH?.endsWith(processEnv.PATH)) { // Prefer prepending to PATH instead of replacing it, as we do not want to replace any // changes to PATH users might have made it in their init scripts (~/.bashrc etc.) @@ -226,7 +230,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ value = `${deactivate}${this.separator}${value}`; } traceVerbose(`Prepending environment variable ${key} in collection with ${value}`); - envVarCollection.prepend(key, value, prependOptions); + envVarCollection.prepend(key, value, options); } else { if (!value.endsWith(this.separator)) { value = value.concat(this.separator); @@ -235,7 +239,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ value = `${deactivate}${this.separator}${value}`; } traceVerbose(`Prepending environment variable ${key} in collection to ${value}`); - envVarCollection.prepend(key, value, prependOptions); + envVarCollection.prepend(key, value, options); } return; } @@ -300,7 +304,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 should be set but no PS1 was set. return; } - const config = await this.shellIntegrationService.isWorking(shell); + const config = await this.shellIntegrationService.isWorking(); if (!config) { traceVerbose('PS1 is not set when shell integration is disabled.'); return; @@ -356,7 +360,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } private async getPrependOptions(): Promise { - const isActive = await this.shellIntegrationService.isWorking(this.applicationEnvironment.shell); + const isActive = await this.shellIntegrationService.isWorking(); // Ideally we would want to prepend exactly once, either at shell integration or process creation. // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available. return isActive diff --git a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts index 9846b0c36f23..03b7d25de986 100644 --- a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts +++ b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts @@ -97,7 +97,8 @@ export class ShellIntegrationService implements IShellIntegrationService { public readonly onDidChangeStatus = this.didChange.event; - public async isWorking(shell: string): Promise { + public async isWorking(): Promise { + const { shell } = this.appEnvironment; return this._isWorking(shell).catch((ex) => { traceError(`Failed to determine if shell supports shell integration`, shell, ex); return false; diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index fbdd490c175c..4c73da63dd1e 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -45,7 +45,7 @@ export interface ITerminalEnvVarCollectionService { export const IShellIntegrationService = Symbol('IShellIntegrationService'); export interface IShellIntegrationService { onDidChangeStatus: Event; - isWorking(shell: string): Promise; + isWorking(): Promise; } export const ITerminalDeactivateService = Symbol('ITerminalDeactivateService'); diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index c1955c2704e6..251c0d1aabaf 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -75,7 +75,7 @@ suite('Terminal Environment Variable Collection Service', () => { context = mock(); shell = mock(); shellIntegrationService = mock(); - when(shellIntegrationService.isWorking(anything())).thenResolve(true); + when(shellIntegrationService.isWorking()).thenResolve(true); globalCollection = mock(); collection = mock(); when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); @@ -336,7 +336,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', prependedPart, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Also prepend deactivate script location if available', async () => { @@ -372,7 +372,7 @@ suite('Terminal Environment Variable Collection Service', () => { const separator = getOSType() === OSType.Windows ? ';' : ':'; verify(collection.prepend('PATH', `scriptLocation${separator}${prependedPart}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Prepend full PATH with separator otherwise', async () => { @@ -405,7 +405,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', `${finalPath}${separator}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Prepend full PATH with separator otherwise', async () => { @@ -441,7 +441,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', `scriptLocation${separator}${finalPath}${separator}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Verify envs are not applied if env activation is disabled', async () => { @@ -523,7 +523,7 @@ suite('Terminal Environment Variable Collection Service', () => { test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => { reset(shellIntegrationService); - when(shellIntegrationService.isWorking(anything())).thenResolve(false); + when(shellIntegrationService.isWorking()).thenResolve(false); when(platform.osType).thenReturn(OSType.Linux); const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env }; const ps1Shell = 'bash';