From 1637a13ecf14398c1bdb104db9530db2871e2aea Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 2 Feb 2023 15:00:42 +0530 Subject: [PATCH] Fix wording in conda inherit env prompt --- src/client/common/utils/localize.ts | 4 +- .../virtualEnvs/condaInheritEnvPrompt.ts | 9 +-- src/client/telemetry/index.ts | 7 +- .../condaInheritEnvPrompt.unit.test.ts | 70 ++----------------- 4 files changed, 13 insertions(+), 77 deletions(-) diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 9680a30a460b..6c8dbcf06663 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -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'); @@ -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?', diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index da3d22a72bee..cf9175345cb0 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -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'; @@ -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, @@ -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, @@ -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'); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c833922ace30..50947d8456e9 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -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: diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index 6ae070721475..9499b5294d78 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -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'; @@ -31,7 +31,6 @@ suite('Conda Inherit Env Prompt', async () => { let appShell: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let platformService: TypeMoq.IMock; - let browserService: TypeMoq.IMock; let applicationEnvironment: TypeMoq.IMock; let persistentStateFactory: IPersistentStateFactory; let notificationPromptEnabled: TypeMoq.IMock>; @@ -46,7 +45,6 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); - browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); platformService = TypeMoq.Mock.ofType(); @@ -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, ); @@ -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, @@ -260,7 +254,6 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); - browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); platformService = TypeMoq.Mock.ofType(); @@ -279,7 +272,6 @@ suite('Conda Inherit Env Prompt', async () => { condaInheritEnvPrompt = new CondaInheritEnvPrompt( interpreterService.object, workspaceService.object, - browserService.object, appShell.object, instance(persistentStateFactory), @@ -305,7 +297,6 @@ suite('Conda Inherit Env Prompt', async () => { condaInheritEnvPrompt = new CondaInheritEnvPrompt( interpreterService.object, workspaceService.object, - browserService.object, appShell.object, instance(persistentStateFactory), @@ -323,7 +314,6 @@ suite('Conda Inherit Env Prompt', async () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); - browserService = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); platformService = TypeMoq.Mock.ofType(); @@ -343,7 +333,6 @@ suite('Conda Inherit Env Prompt', async () => { condaInheritEnvPrompt = new CondaInheritEnvPrompt( interpreterService.object, workspaceService.object, - browserService.object, appShell.object, instance(persistentStateFactory), @@ -363,7 +352,6 @@ suite('Conda Inherit Env Prompt', async () => { condaInheritEnvPrompt = new CondaInheritEnvPrompt( interpreterService.object, workspaceService.object, - browserService.object, appShell.object, instance(persistentStateFactory), @@ -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(); appShell = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); persistentStateFactory = mock(PersistentStateFactory); - browserService = TypeMoq.Mock.ofType(); notificationPromptEnabled = TypeMoq.Mock.ofType>(); platformService = TypeMoq.Mock.ofType(); applicationEnvironment = TypeMoq.Mock.ofType(); @@ -394,7 +381,6 @@ suite('Conda Inherit Env Prompt', async () => { condaInheritEnvPrompt = new CondaInheritEnvPrompt( interpreterService.object, workspaceService.object, - browserService.object, appShell.object, instance(persistentStateFactory), @@ -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(); @@ -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')) @@ -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(); @@ -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(); - 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')) @@ -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(); }); }); });