Skip to content

Commit

Permalink
Make sure PATH ends with a separator before prepending (#22046)
Browse files Browse the repository at this point in the history
Introduced with #21906
For #20950 
Fixes #22047
  • Loading branch information
Kartik Raj authored Sep 21, 2023
1 parent 849be34 commit 42cdaf3
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
*/
private processEnvVars: EnvironmentVariables | undefined;

private separator: string;

constructor(
@inject(IPlatformService) private readonly platform: IPlatformService,
@inject(IInterpreterService) private interpreterService: IInterpreterService,
Expand All @@ -75,7 +77,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
@inject(IPathUtils) private readonly pathUtils: IPathUtils,
) {}
) {
this.separator = platform.osType === OSType.Windows ? ';' : ':';
}

public async activate(resource: Resource): Promise<void> {
try {
Expand Down Expand Up @@ -196,6 +200,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
applyAtProcessCreation: true,
});
} else {
if (!value.endsWith(this.separator)) {
value = value.concat(this.separator);
}
traceVerbose(`Prepending environment variable ${key} in collection to ${value}`);
envVarCollection.prepend(key, value, {
applyAtShellIntegration: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,13 @@ suite('Terminal Environment Variable Collection Service', () => {
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
});

test('Prepend full PATH otherwise', async () => {
test('Prepend full PATH with separator otherwise', async () => {
const processEnv = { PATH: 'hello/1/2/3' };
reset(environmentActivationService);
when(environmentActivationService.getProcessEnvironmentVariables(anything(), anything())).thenResolve(
processEnv,
);
const separator = getOSType() === OSType.Windows ? ';' : ':';
const finalPath = 'hello/3/2/1';
const envVars: NodeJS.ProcessEnv = { PATH: finalPath };
when(
Expand All @@ -275,7 +276,7 @@ suite('Terminal Environment Variable Collection Service', () => {
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);

verify(collection.clear()).once();
verify(collection.prepend('PATH', finalPath, anything())).once();
verify(collection.prepend('PATH', `${finalPath}${separator}`, anything())).once();
verify(collection.replace('PATH', anything(), anything())).never();
assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
});
Expand Down

0 comments on commit 42cdaf3

Please sign in to comment.