Make sure PATH ends with a separator before prepending (microsoft#2…

…2046) Introduced with microsoft#21906 For microsoft#20950 Fixes microsoft#22047
eleanorjboyd · Oct 2, 2023 · a9b4f0d · a9b4f0d
1 parent d917b9c
commit a9b4f0d
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 3 deletions.
diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts
@@ -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,
@@ -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 {
@@ -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,

diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
@@ -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(
@@ -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 });
     });