Skip to content

Commit

Permalink
Call out that env name may not show in terminal activation notificati…
Browse files Browse the repository at this point in the history 
…on (microsoft#21897)

Closes microsoft#21887
  • Loading branch information
@anthonykim1
Kartik Raj authored and anthonykim1 committed Sep 12, 2023
1 parent ff4d75c commit 8d15991
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/client/common/utils/localize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export namespace Interpreters {
export const activatingTerminals = l10n.t('Reactivating terminals...');
export const activateTerminalDescription = l10n.t('Activated environment for');
export const terminalEnvVarCollectionPrompt = l10n.t(
'The Python extension automatically activates all terminals using the selected environment. You can hover over the terminal tab to see more information about the activation. [Learn more](https://aka.ms/vscodePythonTerminalActivation).',
'The Python extension automatically activates all terminals using the selected environment, even when the name of the environment{0} is not present in the terminal prompt. [Learn more](https://aka.ms/vscodePythonTerminalActivation).',
);
export const activatedCondaEnvLaunch = l10n.t(
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
Expand Down
11 changes: 8 additions & 3 deletions src/client/interpreter/activation/terminalEnvVarCollectionPrompt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import {
IDisposableRegistry,
IExperimentService,
IPersistentStateFactory,
Resource,
} from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { ITerminalEnvVarCollectionService } from './types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { IInterpreterService } from '../contracts';

export const terminalEnvCollectionPromptKey = 'TERMINAL_ENV_COLLECTION_PROMPT_KEY';

Expand All @@ -30,6 +32,7 @@ export class TerminalEnvVarCollectionPrompt implements IExtensionSingleActivatio
@inject(ITerminalEnvVarCollectionService)
private readonly terminalEnvVarCollectionService: ITerminalEnvVarCollectionService,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {}

Expand All @@ -52,12 +55,12 @@ export class TerminalEnvVarCollectionPrompt implements IExtensionSingleActivatio
// No need to show notification if terminal prompt already indicates when env is activated.
return;
}
await this.notifyUsers();
await this.notifyUsers(resource);
}),
);
}

private async notifyUsers(): Promise<void> {
private async notifyUsers(resource: Resource): Promise<void> {
const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(
terminalEnvCollectionPromptKey,
true,
Expand All @@ -66,8 +69,10 @@ export class TerminalEnvVarCollectionPrompt implements IExtensionSingleActivatio
return;
}
const prompts = [Common.doNotShowAgain];
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
const terminalPromptName = interpreter?.envName ? ` (${interpreter.envName})` : '';
const selection = await this.appShell.showInformationMessage(
Interpreters.terminalEnvVarCollectionPrompt,
Interpreters.terminalEnvVarCollectionPrompt.format(terminalPromptName),
...prompts,
);
if (!selection) {
Expand Down
37 changes: 24 additions & 13 deletions src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { ITerminalEnvVarCollectionService } from '../../../client/interpreter/ac
import { Common, Interpreters } from '../../../client/common/utils/localize';
import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups';
import { sleep } from '../../core';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';

suite('Terminal Environment Variable Collection Prompt', () => {
let shell: IApplicationShell;
Expand All @@ -30,12 +32,18 @@ suite('Terminal Environment Variable Collection Prompt', () => {
let terminalEventEmitter: EventEmitter<Terminal>;
let notificationEnabled: IPersistentState<boolean>;
let configurationService: IConfigurationService;
let interpreterService: IInterpreterService;
const prompts = [Common.doNotShowAgain];
const message = Interpreters.terminalEnvVarCollectionPrompt;
const envName = 'env';
const expectedMessage = Interpreters.terminalEnvVarCollectionPrompt.format(` (${envName})`);

setup(async () => {
shell = mock<IApplicationShell>();
terminalManager = mock<ITerminalManager>();
interpreterService = mock<IInterpreterService>();
when(interpreterService.getActiveInterpreter(anything())).thenResolve(({
envName,
} as unknown) as PythonEnvironment);
experimentService = mock<IExperimentService>();
activeResourceService = mock<IActiveResourceService>();
persistentStateFactory = mock<IPersistentStateFactory>();
Expand All @@ -61,6 +69,7 @@ suite('Terminal Environment Variable Collection Prompt', () => {
instance(activeResourceService),
instance(terminalEnvVarCollectionService),
instance(configurationService),
instance(interpreterService),
instance(experimentService),
);
});
Expand All @@ -74,13 +83,13 @@ suite('Terminal Environment Variable Collection Prompt', () => {
} as unknown) as Terminal;
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(true);
when(shell.showInformationMessage(message, ...prompts)).thenResolve(undefined);
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenResolve(undefined);

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
await sleep(1);

verify(shell.showInformationMessage(message, ...prompts)).once();
verify(shell.showInformationMessage(expectedMessage, ...prompts)).once();
});

test('Do not show notification if automatic terminal activation is turned off', async () => {
Expand All @@ -98,13 +107,13 @@ suite('Terminal Environment Variable Collection Prompt', () => {
} as unknown) as Terminal;
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(true);
when(shell.showInformationMessage(message, ...prompts)).thenResolve(undefined);
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenResolve(undefined);

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
await sleep(1);

verify(shell.showInformationMessage(message, ...prompts)).never();
verify(shell.showInformationMessage(expectedMessage, ...prompts)).never();
});

test('When not in experiment, do not show notification for the same', async () => {
Expand All @@ -116,15 +125,15 @@ suite('Terminal Environment Variable Collection Prompt', () => {
} as unknown) as Terminal;
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(true);
when(shell.showInformationMessage(message, ...prompts)).thenResolve(undefined);
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenResolve(undefined);

reset(experimentService);
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false);
await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
await sleep(1);

verify(shell.showInformationMessage(message, ...prompts)).never();
verify(shell.showInformationMessage(expectedMessage, ...prompts)).never();
});

test('Do not show notification if notification is disabled', async () => {
Expand All @@ -136,13 +145,13 @@ suite('Terminal Environment Variable Collection Prompt', () => {
} as unknown) as Terminal;
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(false);
when(shell.showInformationMessage(message, ...prompts)).thenResolve(undefined);
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenResolve(undefined);

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
await sleep(1);

verify(shell.showInformationMessage(message, ...prompts)).never();
verify(shell.showInformationMessage(expectedMessage, ...prompts)).never();
});

test('Do not show notification when a new terminal is opened for which there is prompt set', async () => {
Expand All @@ -154,13 +163,13 @@ suite('Terminal Environment Variable Collection Prompt', () => {
} as unknown) as Terminal;
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(true);
when(notificationEnabled.value).thenReturn(true);
when(shell.showInformationMessage(message, ...prompts)).thenResolve(undefined);
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenResolve(undefined);

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
await sleep(1);

verify(shell.showInformationMessage(message, ...prompts)).never();
verify(shell.showInformationMessage(expectedMessage, ...prompts)).never();
});

test("Disable notification if `Don't show again` is clicked", async () => {
Expand All @@ -173,7 +182,9 @@ suite('Terminal Environment Variable Collection Prompt', () => {
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(true);
when(notificationEnabled.updateValue(false)).thenResolve();
when(shell.showInformationMessage(message, ...prompts)).thenReturn(Promise.resolve(Common.doNotShowAgain));
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenReturn(
Promise.resolve(Common.doNotShowAgain),
);

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
Expand All @@ -192,7 +203,7 @@ suite('Terminal Environment Variable Collection Prompt', () => {
when(terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)).thenReturn(false);
when(notificationEnabled.value).thenReturn(true);
when(notificationEnabled.updateValue(false)).thenResolve();
when(shell.showInformationMessage(message, ...prompts)).thenReturn(Promise.resolve(undefined));
when(shell.showInformationMessage(expectedMessage, ...prompts)).thenReturn(Promise.resolve(undefined));

await terminalEnvVarCollectionPrompt.activate();
terminalEventEmitter.fire(terminal);
Expand Down

0 comments on commit 8d15991

Please sign in to comment.