Show notification reaffirming Python extension still handles activation when in pythonTerminalEnvVarActivation experiment (#21802)

Kartik Raj · web-flow · commit 9c740b9557d6 · 2023-08-14T17:50:34.000-07:00
Closes #21793 Only show notification when terminal prompt does not already indicate that env is activated.
diff --git a/src/client/activation/extensionSurvey.ts b/src/client/activation/extensionSurvey.ts
@@ -83,10 +83,10 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
     @traceDecoratorError('Failed to display prompt for extension survey')
     public async showSurvey() {
         const prompts = [ExtensionSurveyBanner.bannerLabelYes, ExtensionSurveyBanner.maybeLater, Common.doNotShowAgain];
-        const telemetrySelections: ['Yes', 'Maybe later', 'Do not show again'] = [
+        const telemetrySelections: ['Yes', 'Maybe later', "Don't show again"] = [
             'Yes',
             'Maybe later',
-            'Do not show again',
+            "Don't show again",
         ];
         const selection = await this.appShell.showInformationMessage(ExtensionSurveyBanner.bannerMessage, ...prompts);
         sendTelemetryEvent(EventName.EXTENSION_SURVEY_PROMPT, undefined, {
diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts
@@ -3,10 +3,16 @@
 
 'use strict';
 
+import { env, workspace } from 'vscode';
 import { IExperimentService } from '../types';
 import { TerminalEnvVarActivation } from './groups';
+import { isTestExecution } from '../constants';
 
 export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean {
+    if (!isTestExecution() && workspace.workspaceFile && env.remoteName) {
+        // TODO: Remove this if statement once https://github.com/microsoft/vscode/issues/180486 is fixed.
+        return false;
+    }
     if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) {
         return false;
     }
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -63,7 +63,7 @@ export namespace Common {
     export const openOutputPanel = l10n.t('Show output');
     export const noIWillDoItLater = l10n.t('No, I will do it later');
     export const notNow = l10n.t('Not now');
-    export const doNotShowAgain = l10n.t('Do not show again');
+    export const doNotShowAgain = l10n.t("Don't show again");
     export const reload = l10n.t('Reload');
     export const moreInfo = l10n.t('More Info');
     export const learnMore = l10n.t('Learn more');
@@ -198,6 +198,9 @@ 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).',
+    );
     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/activation/terminalEnvVarCollectionPrompt.ts b/src/client/interpreter/activation/terminalEnvVarCollectionPrompt.ts
@@ -0,0 +1,80 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { inject, injectable } from 'inversify';
+import { Uri } from 'vscode';
+import { IActiveResourceService, IApplicationShell, ITerminalManager } from '../../common/application/types';
+import {
+    IConfigurationService,
+    IDisposableRegistry,
+    IExperimentService,
+    IPersistentStateFactory,
+} 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';
+
+export const terminalEnvCollectionPromptKey = 'TERMINAL_ENV_COLLECTION_PROMPT_KEY';
+
+@injectable()
+export class TerminalEnvVarCollectionPrompt implements IExtensionSingleActivationService {
+    public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false };
+
+    constructor(
+        @inject(IApplicationShell) private readonly appShell: IApplicationShell,
+        @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
+        @inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
+        @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry,
+        @inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
+        @inject(ITerminalEnvVarCollectionService)
+        private readonly terminalEnvVarCollectionService: ITerminalEnvVarCollectionService,
+        @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
+        @inject(IExperimentService) private readonly experimentService: IExperimentService,
+    ) {}
+
+    public async activate(): Promise<void> {
+        if (!inTerminalEnvVarExperiment(this.experimentService)) {
+            return;
+        }
+        this.disposableRegistry.push(
+            this.terminalManager.onDidOpenTerminal(async (terminal) => {
+                const cwd =
+                    'cwd' in terminal.creationOptions && terminal.creationOptions.cwd
+                        ? terminal.creationOptions.cwd
+                        : this.activeResourceService.getActiveResource();
+                const resource = typeof cwd === 'string' ? Uri.file(cwd) : cwd;
+                const settings = this.configurationService.getSettings(resource);
+                if (!settings.terminal.activateEnvironment) {
+                    return;
+                }
+                if (this.terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)) {
+                    // No need to show notification if terminal prompt already indicates when env is activated.
+                    return;
+                }
+                await this.notifyUsers();
+            }),
+        );
+    }
+
+    private async notifyUsers(): Promise<void> {
+        const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(
+            terminalEnvCollectionPromptKey,
+            true,
+        );
+        if (!notificationPromptEnabled.value) {
+            return;
+        }
+        const prompts = [Common.doNotShowAgain];
+        const selection = await this.appShell.showInformationMessage(
+            Interpreters.terminalEnvVarCollectionPrompt,
+            ...prompts,
+        );
+        if (!selection) {
+            return;
+        }
+        if (selection === prompts[0]) {
+            await notificationPromptEnabled.updateValue(false);
+        }
+    }
+}
diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts
@@ -23,13 +23,15 @@ import { Interpreters } from '../../common/utils/localize';
 import { traceDecoratorVerbose, traceVerbose } from '../../logging';
 import { IInterpreterService } from '../contracts';
 import { defaultShells } from './service';
-import { IEnvironmentActivationService } from './types';
+import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
 import { EnvironmentType } from '../../pythonEnvironments/info';
 import { getSearchPathEnvVarNames } from '../../common/utils/exec';
 import { EnvironmentVariables } from '../../common/variables/types';
+import { TerminalShellType } from '../../common/terminal/types';
+import { OSType } from '../../common/utils/platform';
 
 @injectable()
-export class TerminalEnvVarCollectionService implements IExtensionActivationService {
+export class TerminalEnvVarCollectionService implements IExtensionActivationService, ITerminalEnvVarCollectionService {
     public readonly supportedWorkspaceTypes = {
         untrustedWorkspace: false,
         virtualWorkspace: false,
@@ -127,6 +129,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                 await this._applyCollection(resource, defaultShell?.shell);
                 return;
             }
+            await this.trackTerminalPrompt(shell, resource, env);
             this.processEnvVars = undefined;
             return;
         }
@@ -146,6 +149,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
             if (prevValue !== value) {
                 if (value !== undefined) {
                     if (key === 'PS1') {
+                        // We cannot have the full PS1 without executing in terminal, which we do not. Hence prepend it.
                         traceVerbose(`Prepending environment variable ${key} in collection with ${value}`);
                         envVarCollection.prepend(key, value, {
                             applyAtShellIntegration: true,
@@ -165,6 +169,61 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
         const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath);
         const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``);
         envVarCollection.description = description;
+
+        await this.trackTerminalPrompt(shell, resource, env);
+    }
+
+    private isPromptSet = new Map<number | undefined, boolean>();
+
+    // eslint-disable-next-line class-methods-use-this
+    public isTerminalPromptSetCorrectly(resource?: Resource): boolean {
+        const workspaceFolder = this.getWorkspaceFolder(resource);
+        return !!this.isPromptSet.get(workspaceFolder?.index);
+    }
+
+    /**
+     * Call this once we know terminal prompt is set correctly for terminal owned by this resource.
+     */
+    private terminalPromptIsCorrect(resource: Resource) {
+        const key = this.getWorkspaceFolder(resource)?.index;
+        this.isPromptSet.set(key, true);
+    }
+
+    private terminalPromptIsUnknown(resource: Resource) {
+        const key = this.getWorkspaceFolder(resource)?.index;
+        this.isPromptSet.delete(key);
+    }
+
+    /**
+     * Tracks whether prompt for terminal was correctly set.
+     */
+    private async trackTerminalPrompt(shell: string, resource: Resource, env: EnvironmentVariables | undefined) {
+        this.terminalPromptIsUnknown(resource);
+        if (!env) {
+            this.terminalPromptIsCorrect(resource);
+            return;
+        }
+        // Prompts for these shells cannot be set reliably using variables
+        const exceptionShells = [
+            TerminalShellType.powershell,
+            TerminalShellType.powershellCore,
+            TerminalShellType.fish,
+            TerminalShellType.zsh, // TODO: Remove this once https://github.com/microsoft/vscode/issues/188875 is fixed
+        ];
+        const customShellType = identifyShellFromShellPath(shell);
+        if (exceptionShells.includes(customShellType)) {
+            return;
+        }
+        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 shouldPS1BeSet = interpreter?.type !== undefined;
+            if (shouldPS1BeSet && !env.PS1) {
+                // PS1 should be set but no PS1 was set.
+                return;
+            }
+        }
+        this.terminalPromptIsCorrect(resource);
     }
 
     private async handleMicroVenv(resource: Resource) {
@@ -178,7 +237,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                 envVarCollection.replace(
                     'PATH',
                     `${path.dirname(interpreter.path)}${path.delimiter}${process.env[pathVarName]}`,
-                    { applyAtShellIntegration: true },
+                    { applyAtShellIntegration: true, applyAtProcessCreation: true },
                 );
                 return;
             }
diff --git a/src/client/interpreter/activation/types.ts b/src/client/interpreter/activation/types.ts
@@ -21,3 +21,11 @@ export interface IEnvironmentActivationService {
         interpreter?: PythonEnvironment,
     ): Promise<string[] | undefined>;
 }
+
+export const ITerminalEnvVarCollectionService = Symbol('ITerminalEnvVarCollectionService');
+export interface ITerminalEnvVarCollectionService {
+    /**
+     * Returns true if we know with high certainity the terminal prompt is set correctly for a particular resource.
+     */
+    isTerminalPromptSetCorrectly(resource?: Resource): boolean;
+}
diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts
@@ -6,8 +6,9 @@
 import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types';
 import { IServiceManager } from '../ioc/types';
 import { EnvironmentActivationService } from './activation/service';
+import { TerminalEnvVarCollectionPrompt } from './activation/terminalEnvVarCollectionPrompt';
 import { TerminalEnvVarCollectionService } from './activation/terminalEnvVarCollectionService';
-import { IEnvironmentActivationService } from './activation/types';
+import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './activation/types';
 import { InterpreterAutoSelectionService } from './autoSelection/index';
 import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy';
 import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './autoSelection/types';
@@ -109,8 +110,13 @@ export function registerTypes(serviceManager: IServiceManager): void {
         IEnvironmentActivationService,
         EnvironmentActivationService,
     );
-    serviceManager.addSingleton<IExtensionActivationService>(
-        IExtensionActivationService,
+    serviceManager.addSingleton<ITerminalEnvVarCollectionService>(
+        ITerminalEnvVarCollectionService,
         TerminalEnvVarCollectionService,
     );
+    serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
+    serviceManager.addSingleton<IExtensionSingleActivationService>(
+        IExtensionSingleActivationService,
+        TerminalEnvVarCollectionPrompt,
+    );
 }
diff --git a/src/client/pythonEnvironments/info/index.ts b/src/client/pythonEnvironments/info/index.ts
@@ -4,6 +4,7 @@
 'use strict';
 
 import { Architecture } from '../../common/utils/platform';
+import { PythonEnvType } from '../base/info';
 import { PythonVersion } from './pythonVersion';
 
 /**
@@ -85,7 +86,7 @@ export type PythonEnvironment = InterpreterInformation & {
     envName?: string;
     envPath?: string;
     cachedEntry?: boolean;
-    type?: string;
+    type?: PythonEnvType;
 };
 
 /**
diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts
@@ -950,7 +950,7 @@ export interface IEventNamePropertyMapping {
         tool?: LinterId;
         /**
          * `select` When 'Select linter' option is selected
-         * `disablePrompt` When 'Do not show again' option is selected
+         * `disablePrompt` When "Don't show again" option is selected
          * `install` When 'Install' option is selected
          *
          * @type {('select' | 'disablePrompt' | 'install')}
@@ -1374,7 +1374,7 @@ export interface IEventNamePropertyMapping {
         /**
          * `Yes` When 'Yes' option is selected
          * `No` When 'No' option is selected
-         * `Ignore` When 'Do not show again' option is clicked
+         * `Ignore` When "Don't show again" option is clicked
          *
          * @type {('Yes' | 'No' | 'Ignore' | undefined)}
          */
@@ -1571,7 +1571,7 @@ export interface IEventNamePropertyMapping {
         /**
          * Carries the selection of user when they are asked to take the extension survey
          */
-        selection: 'Yes' | 'Maybe later' | 'Do not show again' | undefined;
+        selection: 'Yes' | 'Maybe later' | "Don't show again" | undefined;
     };
     /**
      * Telemetry event sent when starting REPL
diff --git a/src/test/activation/extensionSurvey.unit.test.ts b/src/test/activation/extensionSurvey.unit.test.ts
@@ -355,7 +355,7 @@ suite('Extension survey prompt - showSurvey()', () => {
         platformService.verifyAll();
     });
 
-    test("Disable prompt if 'Do not show again' option is clicked", async () => {
+    test('Disable prompt if "Don\'t show again" option is clicked', async () => {
         const prompts = [ExtensionSurveyBanner.bannerLabelYes, ExtensionSurveyBanner.maybeLater, Common.doNotShowAgain];
         platformService.setup((p) => p.osType).verifiable(TypeMoq.Times.never());
         appShell
diff --git a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts
@@ -220,7 +220,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
             expect(messagePrompt).not.be.equal(undefined, 'Message prompt not set');
             expect(messagePrompt!.commandPrompts).to.be.deep.equal([
                 { prompt: 'Select Python Interpreter', command: cmd },
-                { prompt: 'Do not show again', command: cmdIgnore },
+                { prompt: "Don't show again", command: cmdIgnore },
             ]);
         });
         test('Should not display a message if No Interpreters diagnostic has been ignored', async () => {
diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts
@@ -307,10 +307,10 @@ suite('Module Installer only', () => {
                                         TypeMoq.It.isAnyString(),
                                         TypeMoq.It.isValue('Install'),
                                         TypeMoq.It.isValue('Select Linter'),
-                                        TypeMoq.It.isValue('Do not show again'),
+                                        TypeMoq.It.isValue("Don't show again"),
                                     ),
                                 )
-                                    .returns(async () => 'Do not show again')
+                                    .returns(async () => "Don't show again")
                                     .verifiable(TypeMoq.Times.once());
                                 const persistVal = TypeMoq.Mock.ofType<IPersistentState<boolean>>();
                                 let mockPersistVal = false;
@@ -367,7 +367,7 @@ suite('Module Installer only', () => {
                                         TypeMoq.It.isAnyString(),
                                         TypeMoq.It.isValue('Install'),
                                         TypeMoq.It.isValue('Select Linter'),
-                                        TypeMoq.It.isValue('Do not show again'),
+                                        TypeMoq.It.isValue("Don't show again"),
                                     ),
                                 )
                                     .returns(async () => undefined)
@@ -864,7 +864,7 @@ suite('Module Installer only', () => {
 
         test('Ensure 3 options for pylint', async () => {
             const product = Product.pylint;
-            const options = ['Select Linter', 'Do not show again'];
+            const options = ['Select Linter', "Don't show again"];
             const productName = ProductNames.get(product)!;
 
             await installer.promptToInstallImplementation(product, resource);
@@ -875,7 +875,7 @@ suite('Module Installer only', () => {
         });
         test('Ensure select linter command is invoked', async () => {
             const product = Product.pylint;
-            const options = ['Select Linter', 'Do not show again'];
+            const options = ['Select Linter', "Don't show again"];
             const productName = ProductNames.get(product)!;
             when(
                 appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]),
@@ -892,7 +892,7 @@ suite('Module Installer only', () => {
         });
         test('If install button is selected, install linter and return response', async () => {
             const product = Product.pylint;
-            const options = ['Select Linter', 'Do not show again'];
+            const options = ['Select Linter', "Don't show again"];
             const productName = ProductNames.get(product)!;
             when(
                 appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]),
diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts
diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
diff --git a/src/test/interpreters/virtualEnvs/virtualEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/virtualEnvPrompt.unit.test.ts
