Apply feedback for the create env trigger prompts (#23258)

microsoft · Apr 22, 2024 · 6aaca06 · 6aaca06
1 parent 577e64c
commit 6aaca06
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 127 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -8,7 +8,6 @@
             "request": "launch",
             "runtimeExecutable": "${execPath}",
             "args": ["--extensionDevelopmentPath=${workspaceFolder}"],
-            "stopOnEntry": false,
             "smartStep": true,
             "sourceMaps": true,
             "outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -31,19 +30,11 @@
             "request": "launch",
             "runtimeExecutable": "${execPath}",
             "args": ["--extensionDevelopmentPath=${workspaceFolder}", "${workspaceFolder}/data"],
-            "stopOnEntry": false,
             "smartStep": true,
             "sourceMaps": true,
             "outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
             "preLaunchTask": "Compile"
         },
-        {
-            "name": "Python: Current File",
-            "type": "python",
-            "request": "launch",
-            "program": "${file}",
-            "console": "integratedTerminal"
-        },
         {
             "name": "Tests (Debugger, VS Code, *.test.ts)",
             "type": "extensionHost",
@@ -55,7 +46,6 @@
                 "--extensionDevelopmentPath=${workspaceFolder}",
                 "--extensionTestsPath=${workspaceFolder}/out/test"
             ],
-            "stopOnEntry": false,
             "sourceMaps": true,
             "smartStep": true,
             "outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -83,7 +73,6 @@
                 "VSC_PYTHON_SMOKE_TEST": "1",
                 "TEST_FILES_SUFFIX": "smoke.test"
             },
-            "stopOnEntry": false,
             "sourceMaps": true,
             "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
             "preLaunchTask": "Compile",
@@ -103,7 +92,6 @@
             "env": {
                 "VSC_PYTHON_CI_TEST_GREP": "" // Modify this to run a subset of the single workspace tests
             },
-            "stopOnEntry": false,
             "sourceMaps": true,
             "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
             "preLaunchTask": "Compile",
@@ -123,7 +111,6 @@
             "env": {
                 "VSC_PYTHON_CI_TEST_GREP": "Language Server:"
             },
-            "stopOnEntry": false,
             "sourceMaps": true,
             "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
             "preLaunchTask": "preTestJediLSP",
@@ -140,7 +127,6 @@
                 "--extensionDevelopmentPath=${workspaceFolder}",
                 "--extensionTestsPath=${workspaceFolder}/out/test"
             ],
-            "stopOnEntry": false,
             "sourceMaps": true,
             "smartStep": true,
             "outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -236,19 +222,19 @@
             "program": "${file}",
             "request": "launch",
             "skipFiles": ["<node_internals>/**"],
-            "type": "pwa-node"
+            "type": "node"
         },
         {
             "name": "Python: Current File",
-            "type": "python",
+            "type": "debugpy",
             "justMyCode": true,
             "request": "launch",
             "program": "${file}",
             "console": "integratedTerminal",
             "cwd": "${workspaceFolder}"
         },
         {
-            "name": "Listen",
+            "name": "Python: Attach Listen",
             "type": "debugpy",
             "request": "attach",
             "listen": { "host": "localhost", "port": 5678 },
@@ -267,7 +253,7 @@
     "compounds": [
         {
             "name": "Debug Test Discovery",
-            "configurations": ["Listen", "Extension"]
+            "configurations": ["Python: Attach Listen", "Extension"]
         }
     ]
 }
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -521,8 +521,6 @@ export namespace CreateEnv {
             'A virtual environment is not currently selected for your Python interpreter. Would you like to create a virtual environment?',
         );
         export const createEnvironment = l10n.t('Create');
-        export const disableCheck = l10n.t('Disable');
-        export const disableCheckWorkspace = l10n.t('Disable (Workspace)');
 
         export const globalPipInstallTriggerMessage = l10n.t(
             'You may have installed Python packages into your global environment, which can cause conflicts between package versions. Would you like to create a virtual environment to isolate your dependencies?',

diff --git a/src/client/pythonEnvironments/creation/common/createEnvTriggerUtils.ts b/src/client/pythonEnvironments/creation/common/createEnvTriggerUtils.ts
@@ -10,7 +10,7 @@ import { PVSC_EXTENSION_ID } from '../../../common/constants';
 import { PythonExtension } from '../../../api/types';
 import { traceVerbose } from '../../../logging';
 import { getConfiguration } from '../../../common/vscodeApis/workspaceApis';
-import { getWorkspaceStateValue, updateWorkspaceStateValue } from '../../../common/persistentState';
+import { getWorkspaceStateValue } from '../../../common/persistentState';
 
 export const CREATE_ENV_TRIGGER_SETTING_PART = 'createEnvironment.trigger';
 export const CREATE_ENV_TRIGGER_SETTING = `python.${CREATE_ENV_TRIGGER_SETTING_PART}`;
@@ -90,15 +90,6 @@ export function disableCreateEnvironmentTrigger(): void {
     }
 }
 
-/**
- * Sets trigger to 'off' in workspace persistent state. This disables trigger check
- * for the current workspace only. In multi root case, it is disabled for all folders
- * in the multi root workspace.
- */
-export async function disableWorkspaceCreateEnvironmentTrigger(): Promise<void> {
-    await updateWorkspaceStateValue(CREATE_ENV_TRIGGER_SETTING, 'off');
-}
-
 let _alreadyCreateEnvCriteriaCheck = false;
 /**
  * Run-once wrapper function for the workspace check to prompt to create an environment.

diff --git a/src/client/pythonEnvironments/creation/createEnvironmentTrigger.ts b/src/client/pythonEnvironments/creation/createEnvironmentTrigger.ts
@@ -10,13 +10,12 @@ import {
     shouldPromptToCreateEnv,
     isCreateEnvWorkspaceCheckNotRun,
     disableCreateEnvironmentTrigger,
-    disableWorkspaceCreateEnvironmentTrigger,
 } from './common/createEnvTriggerUtils';
 import { getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis';
 import { traceError, traceInfo, traceVerbose } from '../../logging';
 import { hasPrefixCondaEnv, hasVenv } from './common/commonUtils';
 import { showInformationMessage } from '../../common/vscodeApis/windowApis';
-import { CreateEnv } from '../../common/utils/localize';
+import { Common, CreateEnv } from '../../common/utils/localize';
 import { executeCommand, registerCommand } from '../../common/vscodeApis/commandApis';
 import { Commands } from '../../common/constants';
 import { Resource } from '../../common/types';
@@ -77,8 +76,7 @@ async function createEnvironmentCheckForWorkspace(uri: Uri): Promise<void> {
     const selection = await showInformationMessage(
         CreateEnv.Trigger.workspaceTriggerMessage,
         CreateEnv.Trigger.createEnvironment,
-        CreateEnv.Trigger.disableCheckWorkspace,
-        CreateEnv.Trigger.disableCheck,
+        Common.doNotShowAgain,
     );
 
     if (selection === CreateEnv.Trigger.createEnvironment) {
@@ -87,10 +85,8 @@ async function createEnvironmentCheckForWorkspace(uri: Uri): Promise<void> {
         } catch (error) {
             traceError('CreateEnv Trigger - Error while creating environment: ', error);
         }
-    } else if (selection === CreateEnv.Trigger.disableCheck) {
+    } else if (selection === Common.doNotShowAgain) {
         disableCreateEnvironmentTrigger();
-    } else if (selection === CreateEnv.Trigger.disableCheckWorkspace) {
-        disableWorkspaceCreateEnvironmentTrigger();
     }
 }
 

diff --git a/src/client/pythonEnvironments/creation/globalPipInTerminalTrigger.ts b/src/client/pythonEnvironments/creation/globalPipInTerminalTrigger.ts
@@ -3,12 +3,11 @@ import { CreateEnvOnPipInstallTrigger } from '../../common/experiments/groups';
 import { inExperiment } from '../common/externalDependencies';
 import {
     disableCreateEnvironmentTrigger,
-    disableWorkspaceCreateEnvironmentTrigger,
     isGlobalPythonSelected,
     shouldPromptToCreateEnv,
 } from './common/createEnvTriggerUtils';
 import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/vscodeApis/workspaceApis';
-import { CreateEnv } from '../../common/utils/localize';
+import { Common, CreateEnv } from '../../common/utils/localize';
 import { traceError, traceInfo } from '../../logging';
 import { executeCommand } from '../../common/vscodeApis/commandApis';
 import { Commands, PVSC_EXTENSION_ID } from '../../common/constants';
@@ -46,8 +45,7 @@ export function registerTriggerForPipInTerminal(disposables: Disposable[]): void
                     const selection = await showWarningMessage(
                         CreateEnv.Trigger.globalPipInstallTriggerMessage,
                         CreateEnv.Trigger.createEnvironment,
-                        CreateEnv.Trigger.disableCheckWorkspace,
-                        CreateEnv.Trigger.disableCheck,
+                        Common.doNotShowAgain,
                     );
                     if (selection === CreateEnv.Trigger.createEnvironment) {
                         try {
@@ -69,10 +67,8 @@ export function registerTriggerForPipInTerminal(disposables: Disposable[]): void
                         } catch (error) {
                             traceError('CreateEnv Trigger - Error while creating environment: ', error);
                         }
-                    } else if (selection === CreateEnv.Trigger.disableCheck) {
+                    } else if (selection === Common.doNotShowAgain) {
                         disableCreateEnvironmentTrigger();
-                    } else if (selection === CreateEnv.Trigger.disableCheckWorkspace) {
-                        disableWorkspaceCreateEnvironmentTrigger();
                     }
                 }
             }

diff --git a/src/test/pythonEnvironments/creation/createEnvironmentTrigger.unit.test.ts b/src/test/pythonEnvironments/creation/createEnvironmentTrigger.unit.test.ts
@@ -15,7 +15,7 @@ import {
 import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis';
 import * as commandApis from '../../../client/common/vscodeApis/commandApis';
 import { Commands } from '../../../client/common/constants';
-import { CreateEnv } from '../../../client/common/utils/localize';
+import { Common, CreateEnv } from '../../../client/common/utils/localize';
 
 suite('Create Environment Trigger', () => {
     let shouldPromptToCreateEnvStub: sinon.SinonStub;
@@ -29,7 +29,6 @@ suite('Create Environment Trigger', () => {
     let getWorkspaceFolderStub: sinon.SinonStub;
     let executeCommandStub: sinon.SinonStub;
     let disableCreateEnvironmentTriggerStub: sinon.SinonStub;
-    let disableWorkspaceCreateEnvironmentTriggerStub: sinon.SinonStub;
 
     const workspace1 = {
         uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
@@ -54,10 +53,6 @@ suite('Create Environment Trigger', () => {
 
         executeCommandStub = sinon.stub(commandApis, 'executeCommand');
         disableCreateEnvironmentTriggerStub = sinon.stub(triggerUtils, 'disableCreateEnvironmentTrigger');
-        disableWorkspaceCreateEnvironmentTriggerStub = sinon.stub(
-            triggerUtils,
-            'disableWorkspaceCreateEnvironmentTrigger',
-        );
     });
 
     teardown(() => {
@@ -208,7 +203,6 @@ suite('Create Environment Trigger', () => {
 
         sinon.assert.notCalled(executeCommandStub);
         sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
-        sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
     });
 
     test('Should show prompt if all conditions met: User clicks create', async () => {
@@ -232,18 +226,17 @@ suite('Create Environment Trigger', () => {
 
         sinon.assert.calledOnceWithExactly(executeCommandStub, Commands.Create_Environment);
         sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
-        sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
     });
 
-    test('Should show prompt if all conditions met: User clicks disable global', async () => {
+    test("Should show prompt if all conditions met: User clicks don't show again", async () => {
         shouldPromptToCreateEnvStub.returns(true);
         hasVenvStub.resolves(false);
         hasPrefixCondaEnvStub.resolves(false);
         hasRequirementFilesStub.resolves(true);
         hasKnownFilesStub.resolves(false);
         isGlobalPythonSelectedStub.resolves(true);
 
-        showInformationMessageStub.resolves(CreateEnv.Trigger.disableCheck);
+        showInformationMessageStub.resolves(Common.doNotShowAgain);
         await triggerCreateEnvironmentCheck(CreateEnvironmentCheckKind.Workspace, workspace1.uri);
 
         sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
@@ -256,30 +249,5 @@ suite('Create Environment Trigger', () => {
 
         sinon.assert.notCalled(executeCommandStub);
         sinon.assert.calledOnce(disableCreateEnvironmentTriggerStub);
-        sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
-    });
-
-    test('Should show prompt if all conditions met: User clicks disable workspace', async () => {
-        shouldPromptToCreateEnvStub.returns(true);
-        hasVenvStub.resolves(false);
-        hasPrefixCondaEnvStub.resolves(false);
-        hasRequirementFilesStub.resolves(true);
-        hasKnownFilesStub.resolves(false);
-        isGlobalPythonSelectedStub.resolves(true);
-
-        showInformationMessageStub.resolves(CreateEnv.Trigger.disableCheckWorkspace);
-        await triggerCreateEnvironmentCheck(CreateEnvironmentCheckKind.Workspace, workspace1.uri);
-
-        sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
-        sinon.assert.calledOnce(hasVenvStub);
-        sinon.assert.calledOnce(hasPrefixCondaEnvStub);
-        sinon.assert.calledOnce(hasRequirementFilesStub);
-        sinon.assert.calledOnce(hasKnownFilesStub);
-        sinon.assert.calledOnce(isGlobalPythonSelectedStub);
-        sinon.assert.calledOnce(showInformationMessageStub);
-
-        sinon.assert.notCalled(executeCommandStub);
-        sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
-        sinon.assert.calledOnce(disableWorkspaceCreateEnvironmentTriggerStub);
     });
 });
diff --git a/src/test/pythonEnvironments/creation/globalPipInTerminalTrigger.unit.test.ts b/src/test/pythonEnvironments/creation/globalPipInTerminalTrigger.unit.test.ts
@@ -20,7 +20,7 @@ import * as commandApis from '../../../client/common/vscodeApis/commandApis';
 import * as extDepApi from '../../../client/pythonEnvironments/common/externalDependencies';
 import { registerTriggerForPipInTerminal } from '../../../client/pythonEnvironments/creation/globalPipInTerminalTrigger';
 import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants';
-import { CreateEnv } from '../../../client/common/utils/localize';
+import { Common, CreateEnv } from '../../../client/common/utils/localize';
 
 suite('Global Pip in Terminal Trigger', () => {
     let shouldPromptToCreateEnvStub: sinon.SinonStub;
@@ -31,7 +31,6 @@ suite('Global Pip in Terminal Trigger', () => {
     let showWarningMessageStub: sinon.SinonStub;
     let executeCommandStub: sinon.SinonStub;
     let disableCreateEnvironmentTriggerStub: sinon.SinonStub;
-    let disableWorkspaceCreateEnvironmentTriggerStub: sinon.SinonStub;
     let onDidStartTerminalShellExecutionStub: sinon.SinonStub;
     let handler: undefined | ((e: TerminalShellExecutionStartEvent) => Promise<void>);
     let execEvent: typemoq.IMock<TerminalShellExecutionStartEvent>;
@@ -64,10 +63,6 @@ suite('Global Pip in Terminal Trigger', () => {
         executeCommandStub.resolves({ path: 'some/python' });
 
         disableCreateEnvironmentTriggerStub = sinon.stub(triggerUtils, 'disableCreateEnvironmentTrigger');
-        disableWorkspaceCreateEnvironmentTriggerStub = sinon.stub(
-            triggerUtils,
-            'disableWorkspaceCreateEnvironmentTrigger',
-        );
 
         onDidStartTerminalShellExecutionStub = sinon.stub(windowApis, 'onDidStartTerminalShellExecution');
         onDidStartTerminalShellExecutionStub.callsFake((cb) => {
@@ -270,16 +265,15 @@ suite('Global Pip in Terminal Trigger', () => {
 
         sinon.assert.calledOnce(executeCommandStub);
         sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
-        sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
 
         shellIntegration.verify((s) => s.executeCommand(typemoq.It.isAnyString()), typemoq.Times.once());
     });
 
-    test('Should disable create environment trigger if user selects to disable', async () => {
+    test("Should disable create environment trigger if user selects don't show again", async () => {
         shouldPromptToCreateEnvStub.returns(true);
         inExperimentStub.returns(true);
         isGlobalPythonSelectedStub.returns(true);
-        showWarningMessageStub.resolves(CreateEnv.Trigger.disableCheck);
+        showWarningMessageStub.resolves(Common.doNotShowAgain);
 
         const disposables: Disposable[] = [];
         registerTriggerForPipInTerminal(disposables);
@@ -310,44 +304,5 @@ suite('Global Pip in Terminal Trigger', () => {
 
         sinon.assert.notCalled(executeCommandStub);
         sinon.assert.calledOnce(disableCreateEnvironmentTriggerStub);
-        sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
-    });
-
-    test('Should disable workspace create environment trigger if user selects to disable', async () => {
-        shouldPromptToCreateEnvStub.returns(true);
-        inExperimentStub.returns(true);
-        isGlobalPythonSelectedStub.returns(true);
-        showWarningMessageStub.resolves(CreateEnv.Trigger.disableCheckWorkspace);
-
-        const disposables: Disposable[] = [];
-        registerTriggerForPipInTerminal(disposables);
-
-        await handler?.({
-            terminal: ({} as unknown) as Terminal,
-            shellIntegration: shellIntegration.object,
-            execution: {
-                cwd: workspace1.uri,
-                commandLine: {
-                    isTrusted: true,
-                    value: 'pip install',
-                    confidence: 0,
-                },
-                read: () =>
-                    (async function* () {
-                        yield Promise.resolve('pip install');
-                    })(),
-            },
-        });
-
-        assert.strictEqual(disposables.length, 1);
-        sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
-        sinon.assert.calledOnce(inExperimentStub);
-        sinon.assert.calledOnce(getWorkspaceFolderStub);
-        sinon.assert.calledOnce(isGlobalPythonSelectedStub);
-        sinon.assert.calledOnce(showWarningMessageStub);
-
-        sinon.assert.notCalled(executeCommandStub);
-        sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
-        sinon.assert.calledOnce(disableWorkspaceCreateEnvironmentTriggerStub);
     });
 });