Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5f971ae

Browse filesBrowse files
author
Kartik Raj
authored
Prepend PATH both at shell integration and process creation (#22905)
1 parent 5174d5c commit 5f971ae
Copy full SHA for 5f971ae

File tree

Expand file treeCollapse file tree

4 files changed

+18
-13
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+18
-13
lines changed

‎src/client/terminals/envCollectionActivation/service.ts

Copy file name to clipboardExpand all lines: src/client/terminals/envCollectionActivation/service.ts
+9-5Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
130130
this.disposables,
131131
);
132132
const { shell } = this.applicationEnvironment;
133-
const isActive = await this.shellIntegrationService.isWorking(shell);
133+
const isActive = await this.shellIntegrationService.isWorking();
134134
const shellType = identifyShellFromShellPath(shell);
135135
if (!isActive && shellType !== TerminalShellType.commandPrompt) {
136136
traceWarn(
@@ -218,6 +218,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
218218
return;
219219
}
220220
if (key === 'PATH') {
221+
const options = {
222+
applyAtShellIntegration: true,
223+
applyAtProcessCreation: true,
224+
};
221225
if (processEnv.PATH && env.PATH?.endsWith(processEnv.PATH)) {
222226
// Prefer prepending to PATH instead of replacing it, as we do not want to replace any
223227
// changes to PATH users might have made it in their init scripts (~/.bashrc etc.)
@@ -226,7 +230,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
226230
value = `${deactivate}${this.separator}${value}`;
227231
}
228232
traceVerbose(`Prepending environment variable ${key} in collection with ${value}`);
229-
envVarCollection.prepend(key, value, prependOptions);
233+
envVarCollection.prepend(key, value, options);
230234
} else {
231235
if (!value.endsWith(this.separator)) {
232236
value = value.concat(this.separator);
@@ -235,7 +239,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
235239
value = `${deactivate}${this.separator}${value}`;
236240
}
237241
traceVerbose(`Prepending environment variable ${key} in collection to ${value}`);
238-
envVarCollection.prepend(key, value, prependOptions);
242+
envVarCollection.prepend(key, value, options);
239243
}
240244
return;
241245
}
@@ -300,7 +304,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
300304
// PS1 should be set but no PS1 was set.
301305
return;
302306
}
303-
const config = await this.shellIntegrationService.isWorking(shell);
307+
const config = await this.shellIntegrationService.isWorking();
304308
if (!config) {
305309
traceVerbose('PS1 is not set when shell integration is disabled.');
306310
return;
@@ -356,7 +360,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
356360
}
357361

358362
private async getPrependOptions(): Promise<EnvironmentVariableMutatorOptions> {
359-
const isActive = await this.shellIntegrationService.isWorking(this.applicationEnvironment.shell);
363+
const isActive = await this.shellIntegrationService.isWorking();
360364
// Ideally we would want to prepend exactly once, either at shell integration or process creation.
361365
// TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available.
362366
return isActive

‎src/client/terminals/envCollectionActivation/shellIntegrationService.ts

Copy file name to clipboardExpand all lines: src/client/terminals/envCollectionActivation/shellIntegrationService.ts
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ export class ShellIntegrationService implements IShellIntegrationService {
9797

9898
public readonly onDidChangeStatus = this.didChange.event;
9999

100-
public async isWorking(shell: string): Promise<boolean> {
100+
public async isWorking(): Promise<boolean> {
101+
const { shell } = this.appEnvironment;
101102
return this._isWorking(shell).catch((ex) => {
102103
traceError(`Failed to determine if shell supports shell integration`, shell, ex);
103104
return false;

‎src/client/terminals/types.ts

Copy file name to clipboardExpand all lines: src/client/terminals/types.ts
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export interface ITerminalEnvVarCollectionService {
4545
export const IShellIntegrationService = Symbol('IShellIntegrationService');
4646
export interface IShellIntegrationService {
4747
onDidChangeStatus: Event<void>;
48-
isWorking(shell: string): Promise<boolean>;
48+
isWorking(): Promise<boolean>;
4949
}
5050

5151
export const ITerminalDeactivateService = Symbol('ITerminalDeactivateService');

‎src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

Copy file name to clipboardExpand all lines: src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ suite('Terminal Environment Variable Collection Service', () => {
7575
context = mock<IExtensionContext>();
7676
shell = mock<IApplicationShell>();
7777
shellIntegrationService = mock<IShellIntegrationService>();
78-
when(shellIntegrationService.isWorking(anything())).thenResolve(true);
78+
when(shellIntegrationService.isWorking()).thenResolve(true);
7979
globalCollection = mock<GlobalEnvironmentVariableCollection>();
8080
collection = mock<EnvironmentVariableCollection>();
8181
when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
@@ -336,7 +336,7 @@ suite('Terminal Environment Variable Collection Service', () => {
336336
verify(collection.clear()).once();
337337
verify(collection.prepend('PATH', prependedPart, anything())).once();
338338
verify(collection.replace('PATH', anything(), anything())).never();
339-
assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
339+
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
340340
});
341341

342342
test('Also prepend deactivate script location if available', async () => {
@@ -372,7 +372,7 @@ suite('Terminal Environment Variable Collection Service', () => {
372372
const separator = getOSType() === OSType.Windows ? ';' : ':';
373373
verify(collection.prepend('PATH', `scriptLocation${separator}${prependedPart}`, anything())).once();
374374
verify(collection.replace('PATH', anything(), anything())).never();
375-
assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
375+
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
376376
});
377377

378378
test('Prepend full PATH with separator otherwise', async () => {
@@ -405,7 +405,7 @@ suite('Terminal Environment Variable Collection Service', () => {
405405
verify(collection.clear()).once();
406406
verify(collection.prepend('PATH', `${finalPath}${separator}`, anything())).once();
407407
verify(collection.replace('PATH', anything(), anything())).never();
408-
assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
408+
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
409409
});
410410

411411
test('Prepend full PATH with separator otherwise', async () => {
@@ -441,7 +441,7 @@ suite('Terminal Environment Variable Collection Service', () => {
441441
verify(collection.clear()).once();
442442
verify(collection.prepend('PATH', `scriptLocation${separator}${finalPath}${separator}`, anything())).once();
443443
verify(collection.replace('PATH', anything(), anything())).never();
444-
assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
444+
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
445445
});
446446

447447
test('Verify envs are not applied if env activation is disabled', async () => {
@@ -523,7 +523,7 @@ suite('Terminal Environment Variable Collection Service', () => {
523523

524524
test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
525525
reset(shellIntegrationService);
526-
when(shellIntegrationService.isWorking(anything())).thenResolve(false);
526+
when(shellIntegrationService.isWorking()).thenResolve(false);
527527
when(platform.osType).thenReturn(OSType.Linux);
528528
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
529529
const ps1Shell = 'bash';

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.