Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate PS1 instead of using PS1 returned by shell #22078

Merged
merged 4 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading