Skip to content

Commit

Permalink
Calculate PS1 instead of using PS1 returned by shell (microsoft#22078)
Browse files Browse the repository at this point in the history
Closes microsoft#22056

`PS1` returned by shell is not predictable, it can be `(.venv) ` or
already have the context of the terminal:
```
(venv) [\u@\h \W]\[\e[91m\]$(parse_git_branch)\[\e[00m\]$ 
```
Calculate it to be safe and not double prepend it.
  • Loading branch information
Kartik Raj authored and eleanorjboyd committed Oct 2, 2023
1 parent 461df08 commit 727ade8
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -381,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,21 +191,28 @@ 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 () => {
const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env, PS1: '(prompt)' };
// 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();
}
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;
});

Expand Down

0 comments on commit 727ade8

Please sign in to comment.