Skip to content

Fix wording in conda inherit env prompt #20627

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Fix wording in conda inherit env prompt #20627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/client/common/utils/localize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export namespace Diagnostics {
}

export namespace Common {
export const allow = l10n.t('Allow');
export const close = l10n.t('Close');
export const bannerLabelYes = l10n.t('Yes');
export const bannerLabelNo = l10n.t('No');
export const yesPlease = l10n.t('Yes, please');
Expand Down Expand Up @@ -189,7 +191,7 @@ export namespace Interpreters {
export const discovering = l10n.t('Discovering Python Interpreters');
export const refreshing = l10n.t('Refreshing Python Interpreters');
export const condaInheritEnvMessage = l10n.t(
'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings.',
'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings. [Learn more](https://aka.ms/AA66i8f).',
);
export const activatedCondaEnvLaunch = l10n.t(
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
Expand Down
9 changes: 3 additions & 6 deletions src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ConfigurationTarget, Uri } from 'vscode';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IPlatformService } from '../../common/platform/types';
import { IBrowserService, IPersistentStateFactory } from '../../common/types';
import { IPersistentStateFactory } from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
import { traceDecoratorError, traceError } from '../../logging';
import { EnvironmentType } from '../../pythonEnvironments/info';
Expand All @@ -22,7 +22,6 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
constructor(
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IBrowserService) private browserService: IBrowserService,
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(IPlatformService) private readonly platformService: IPlatformService,
Expand Down Expand Up @@ -52,8 +51,8 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
if (!notificationPromptEnabled.value) {
return;
}
const prompts = [Common.bannerLabelYes, Common.bannerLabelNo, Common.moreInfo];
const telemetrySelections: ['Yes', 'No', 'More Info'] = ['Yes', 'No', 'More Info'];
const prompts = [Common.allow, Common.close];
const telemetrySelections: ['Allow', 'Close'] = ['Allow', 'Close'];
const selection = await this.appShell.showInformationMessage(Interpreters.condaInheritEnvMessage, ...prompts);
sendTelemetryEvent(EventName.CONDA_INHERIT_ENV_PROMPT, undefined, {
selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined,
Expand All @@ -67,8 +66,6 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
.update('integrated.inheritEnv', false, ConfigurationTarget.Global);
} else if (selection === prompts[1]) {
await notificationPromptEnabled.updateValue(false);
} else if (selection === prompts[2]) {
this.browserService.launch('https://aka.ms/AA66i8f');
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/client/telemetry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1310,11 +1310,10 @@ export interface IEventNamePropertyMapping {
*/
[EventName.CONDA_INHERIT_ENV_PROMPT]: {
/**
* `Yes` When 'Yes' option is selected
* `No` When 'No' option is selected
* `More info` When 'More Info' option is selected
* `Yes` When 'Allow' option is selected
* `Close` When 'Close' option is selected
*/
selection: 'Yes' | 'No' | 'More Info' | undefined;
selection: 'Allow' | 'Close' | undefined;
};
/**
* Telemetry event sent with details when user clicks the prompt with the following message:
Expand Down
70 changes: 4 additions & 66 deletions src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from '../../../client/common/application/types';
import { PersistentStateFactory } from '../../../client/common/persistentState';
import { IPlatformService } from '../../../client/common/platform/types';
import { IBrowserService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types';
import { IPersistentState, IPersistentStateFactory } from '../../../client/common/types';
import { createDeferred, createDeferredFromPromise, sleep } from '../../../client/common/utils/async';
import { Common, Interpreters } from '../../../client/common/utils/localize';
import { IInterpreterService } from '../../../client/interpreter/contracts';
Expand All @@ -31,7 +31,6 @@ suite('Conda Inherit Env Prompt', async () => {
let appShell: TypeMoq.IMock<IApplicationShell>;
let interpreterService: TypeMoq.IMock<IInterpreterService>;
let platformService: TypeMoq.IMock<IPlatformService>;
let browserService: TypeMoq.IMock<IBrowserService>;
let applicationEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
let persistentStateFactory: IPersistentStateFactory;
let notificationPromptEnabled: TypeMoq.IMock<IPersistentState<any>>;
Expand All @@ -46,7 +45,6 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
Expand All @@ -55,10 +53,8 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
Expand All @@ -67,10 +63,8 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
true,
Expand Down Expand Up @@ -260,7 +254,6 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
Expand All @@ -279,7 +272,6 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

Expand All @@ -305,7 +297,6 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

Expand All @@ -323,7 +314,6 @@ suite('Conda Inherit Env Prompt', async () => {
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
Expand All @@ -343,7 +333,6 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

Expand All @@ -363,7 +352,6 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

Expand All @@ -377,13 +365,12 @@ suite('Conda Inherit Env Prompt', async () => {
});

suite('Method promptAndUpdate()', () => {
const prompts = [Common.bannerLabelYes, Common.bannerLabelNo, Common.moreInfo];
const prompts = [Common.allow, Common.close];
setup(() => {
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
browserService = TypeMoq.Mock.ofType<IBrowserService>();
notificationPromptEnabled = TypeMoq.Mock.ofType<IPersistentState<any>>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
applicationEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
Expand All @@ -394,7 +381,6 @@ suite('Conda Inherit Env Prompt', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

Expand Down Expand Up @@ -439,16 +425,11 @@ suite('Conda Inherit Env Prompt', async () => {
.setup((n) => n.updateValue(false))
.returns(() => Promise.resolve(undefined))
.verifiable(TypeMoq.Times.never());
browserService
.setup((b) => b.launch('https://aka.ms/AA66i8f'))
.returns(() => undefined)
.verifiable(TypeMoq.Times.never());
await condaInheritEnvPrompt.promptAndUpdate();
verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once();
verifyAll();
workspaceConfig.verifyAll();
notificationPromptEnabled.verifyAll();
browserService.verifyAll();
});
test('Update terminal settings if `Yes` is selected', async () => {
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
Expand All @@ -458,7 +439,7 @@ suite('Conda Inherit Env Prompt', async () => {
.verifiable(TypeMoq.Times.once());
appShell
.setup((a) => a.showInformationMessage(Interpreters.condaInheritEnvMessage, ...prompts))
.returns(() => Promise.resolve(Common.bannerLabelYes))
.returns(() => Promise.resolve(Common.allow))
.verifiable(TypeMoq.Times.once());
workspaceService
.setup((ws) => ws.getConfiguration('terminal'))
Expand All @@ -472,16 +453,11 @@ suite('Conda Inherit Env Prompt', async () => {
.setup((n) => n.updateValue(false))
.returns(() => Promise.resolve(undefined))
.verifiable(TypeMoq.Times.never());
browserService
.setup((b) => b.launch('https://aka.ms/AA66i8f'))
.returns(() => undefined)
.verifiable(TypeMoq.Times.never());
await condaInheritEnvPrompt.promptAndUpdate();
verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once();
verifyAll();
workspaceConfig.verifyAll();
notificationPromptEnabled.verifyAll();
browserService.verifyAll();
});
test('Disable notification prompt if `No` is selected', async () => {
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
Expand All @@ -491,40 +467,7 @@ suite('Conda Inherit Env Prompt', async () => {
.verifiable(TypeMoq.Times.once());
appShell
.setup((a) => a.showInformationMessage(Interpreters.condaInheritEnvMessage, ...prompts))
.returns(() => Promise.resolve(Common.bannerLabelNo))
.verifiable(TypeMoq.Times.once());
workspaceService
.setup((ws) => ws.getConfiguration('terminal'))
.returns(() => workspaceConfig.object)
.verifiable(TypeMoq.Times.never());
workspaceConfig
.setup((wc) => wc.update('integrated.inheritEnv', false, ConfigurationTarget.Global))
.returns(() => Promise.resolve())
.verifiable(TypeMoq.Times.never());
notificationPromptEnabled
.setup((n) => n.updateValue(false))
.returns(() => Promise.resolve(undefined))
.verifiable(TypeMoq.Times.once());
browserService
.setup((b) => b.launch('https://aka.ms/AA66i8f'))
.returns(() => undefined)
.verifiable(TypeMoq.Times.never());
await condaInheritEnvPrompt.promptAndUpdate();
verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once();
verifyAll();
workspaceConfig.verifyAll();
notificationPromptEnabled.verifyAll();
browserService.verifyAll();
});
test('Launch browser if `More info` option is selected', async () => {
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
notificationPromptEnabled
.setup((n) => n.value)
.returns(() => true)
.verifiable(TypeMoq.Times.once());
appShell
.setup((a) => a.showInformationMessage(Interpreters.condaInheritEnvMessage, ...prompts))
.returns(() => Promise.resolve(Common.moreInfo))
.returns(() => Promise.resolve(Common.close))
.verifiable(TypeMoq.Times.once());
workspaceService
.setup((ws) => ws.getConfiguration('terminal'))
Expand All @@ -537,17 +480,12 @@ suite('Conda Inherit Env Prompt', async () => {
notificationPromptEnabled
.setup((n) => n.updateValue(false))
.returns(() => Promise.resolve(undefined))
.verifiable(TypeMoq.Times.never());
browserService
.setup((b) => b.launch('https://aka.ms/AA66i8f'))
.returns(() => undefined)
.verifiable(TypeMoq.Times.once());
await condaInheritEnvPrompt.promptAndUpdate();
verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once();
verifyAll();
workspaceConfig.verifyAll();
notificationPromptEnabled.verifyAll();
browserService.verifyAll();
});
});
});