From 8ea7495cc1a302e588da5ad2a08aa1f4029c6f57 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 25 Sep 2023 19:18:00 +0000 Subject: [PATCH 1/4] Calculate PS1 instead of using PS1 returned by shell --- .../terminalEnvVarCollectionService.ts | 21 ++++++++++++------- ...rminalEnvVarCollectionService.unit.test.ts | 19 ++++++++++------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 50a15d25f3de..d9b761c93b43 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -282,22 +282,25 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } private async getPS1(shell: string, resource: Resource, env: EnvironmentVariables) { - if (env.PS1) { - return env.PS1; - } const customShellType = identifyShellFromShellPath(shell); if (this.noPromptVariableShells.includes(customShellType)) { - return undefined; + return env.PS1; } if (this.platform.osType !== OSType.Windows) { // These shells are expected to set PS1 variable for terminal prompt for virtual/conda environments. const interpreter = await this.interpreterService.getActiveInterpreter(resource); const shouldSetPS1 = shouldPS1BeSet(interpreter?.type, env); - if (shouldSetPS1 && !env.PS1) { - // PS1 should be set but no PS1 was set. - return getPromptForEnv(interpreter); + if (shouldSetPS1) { + const prompt = getPromptForEnv(interpreter); + if (prompt) { + return prompt; + } } } + if (env.PS1) { + // Prefer PS1 set by env vars, as env.PS1 may or may not contain the full PS1: #22056. + return env.PS1; + } return undefined; } @@ -369,6 +372,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } function shouldPS1BeSet(type: PythonEnvType | undefined, env: EnvironmentVariables): boolean { + if (env.PS1) { + // Activated variables contain PS1, meaning it was supposed to be set. + return true; + } if (type === PythonEnvType.Virtual) { const promptDisabledVar = env.VIRTUAL_ENV_DISABLE_PROMPT; const isPromptDisabled = promptDisabledVar && promptDisabledVar !== undefined; diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 091986b7e18b..8f8d45c39195 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -192,20 +192,23 @@ suite('Terminal Environment Variable Collection Service', () => { }); test('If activated variables contain PS1, prefix it using shell integration', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env, PS1: '(prompt)' }; + const envVars: NodeJS.ProcessEnv = { + CONDA_PREFIX: 'prefix/to/conda', + ...process.env, + PS1: '(envName) extra prompt', // Should not use this + }; when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), + environmentActivationService.getActivatedEnvironmentVariables(anything(), undefined, undefined, 'bash'), ).thenResolve(envVars); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + envName: 'envName', + } as unknown) as PythonEnvironment); + when(collection.replace(anything(), anything(), anything())).thenResolve(); when(collection.delete(anything())).thenResolve(); let opts: EnvironmentVariableMutatorOptions | undefined; - when(collection.prepend('PS1', '(prompt)', anything())).thenCall((_, _v, o) => { + when(collection.prepend('PS1', '(envName) ', anything())).thenCall((_, _v, o) => { opts = o; }); From 1022c0118d1c2e1971553d74580707857f657d08 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 25 Sep 2023 19:24:39 +0000 Subject: [PATCH 2/4] Only do it for virtualenv and conda --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index d9b761c93b43..a4a5137953a6 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -388,7 +388,7 @@ function shouldPS1BeSet(type: PythonEnvType | undefined, env: EnvironmentVariabl const isPromptEnabled = promptEnabledVar && promptEnabledVar !== ''; return !!isPromptEnabled; } - return type !== undefined; + return false; } function shouldSkip(env: string) { From 79780ae66688a0ee9dd6007c267ada4fb77d9cbd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 25 Sep 2023 12:38:21 -0700 Subject: [PATCH 3/4] Skip test on windows --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 8f8d45c39195..06cc1e1ac72d 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -191,7 +191,10 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); }); - test('If activated variables contain PS1, prefix it using shell integration', async () => { + test('If activated variables contain PS1, prefix it using shell integration', async function () { + if (getOSType() === OSType.Windows) { + return this.skip(); + } const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env, From 262a04162c2c5c92ec6c11e5132349731ff9a735 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 25 Sep 2023 19:45:06 +0000 Subject: [PATCH 4/4] Fix litn --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 06cc1e1ac72d..e41d6ce4d53c 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -191,6 +191,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); }); + // eslint-disable-next-line consistent-return test('If activated variables contain PS1, prefix it using shell integration', async function () { if (getOSType() === OSType.Windows) { return this.skip();