Skip to content

Commit 6aaca06

Browse files
karthiknadigkarthiknadig
authored
Apply feedback for the create env trigger prompts (#23258)
1 parent 577e64c commit 6aaca06

7 files changed

Lines changed: 17 additions & 127 deletions

File tree

‎.vscode/launch.json‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
"request": "launch",
99
"runtimeExecutable": "${execPath}",
1010
"args": ["--extensionDevelopmentPath=${workspaceFolder}"],
11-
"stopOnEntry": false,
1211
"smartStep": true,
1312
"sourceMaps": true,
1413
"outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -31,19 +30,11 @@
3130
"request": "launch",
3231
"runtimeExecutable": "${execPath}",
3332
"args": ["--extensionDevelopmentPath=${workspaceFolder}", "${workspaceFolder}/data"],
34-
"stopOnEntry": false,
3533
"smartStep": true,
3634
"sourceMaps": true,
3735
"outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
3836
"preLaunchTask": "Compile"
3937
},
40-
{
41-
"name": "Python: Current File",
42-
"type": "python",
43-
"request": "launch",
44-
"program": "${file}",
45-
"console": "integratedTerminal"
46-
},
4738
{
4839
"name": "Tests (Debugger, VS Code, *.test.ts)",
4940
"type": "extensionHost",
@@ -55,7 +46,6 @@
5546
"--extensionDevelopmentPath=${workspaceFolder}",
5647
"--extensionTestsPath=${workspaceFolder}/out/test"
5748
],
58-
"stopOnEntry": false,
5949
"sourceMaps": true,
6050
"smartStep": true,
6151
"outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -83,7 +73,6 @@
8373
"VSC_PYTHON_SMOKE_TEST": "1",
8474
"TEST_FILES_SUFFIX": "smoke.test"
8575
},
86-
"stopOnEntry": false,
8776
"sourceMaps": true,
8877
"outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
8978
"preLaunchTask": "Compile",
@@ -103,7 +92,6 @@
10392
"env": {
10493
"VSC_PYTHON_CI_TEST_GREP": "" // Modify this to run a subset of the single workspace tests
10594
},
106-
"stopOnEntry": false,
10795
"sourceMaps": true,
10896
"outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
10997
"preLaunchTask": "Compile",
@@ -123,7 +111,6 @@
123111
"env": {
124112
"VSC_PYTHON_CI_TEST_GREP": "Language Server:"
125113
},
126-
"stopOnEntry": false,
127114
"sourceMaps": true,
128115
"outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
129116
"preLaunchTask": "preTestJediLSP",
@@ -140,7 +127,6 @@
140127
"--extensionDevelopmentPath=${workspaceFolder}",
141128
"--extensionTestsPath=${workspaceFolder}/out/test"
142129
],
143-
"stopOnEntry": false,
144130
"sourceMaps": true,
145131
"smartStep": true,
146132
"outFiles": ["${workspaceFolder}/out/**/*", "!${workspaceFolder}/**/node_modules**/*"],
@@ -236,19 +222,19 @@
236222
"program": "${file}",
237223
"request": "launch",
238224
"skipFiles": ["<node_internals>/**"],
239-
"type": "pwa-node"
225+
"type": "node"
240226
},
241227
{
242228
"name": "Python: Current File",
243-
"type": "python",
229+
"type": "debugpy",
244230
"justMyCode": true,
245231
"request": "launch",
246232
"program": "${file}",
247233
"console": "integratedTerminal",
248234
"cwd": "${workspaceFolder}"
249235
},
250236
{
251-
"name": "Listen",
237+
"name": "Python: Attach Listen",
252238
"type": "debugpy",
253239
"request": "attach",
254240
"listen": { "host": "localhost", "port": 5678 },
@@ -267,7 +253,7 @@
267253
"compounds": [
268254
{
269255
"name": "Debug Test Discovery",
270-
"configurations": ["Listen", "Extension"]
256+
"configurations": ["Python: Attach Listen", "Extension"]
271257
}
272258
]
273259
}

‎src/client/common/utils/localize.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,8 +521,6 @@ export namespace CreateEnv {
521521
'A virtual environment is not currently selected for your Python interpreter. Would you like to create a virtual environment?',
522522
);
523523
export const createEnvironment = l10n.t('Create');
524-
export const disableCheck = l10n.t('Disable');
525-
export const disableCheckWorkspace = l10n.t('Disable (Workspace)');
526524

527525
export const globalPipInstallTriggerMessage = l10n.t(
528526
'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?',

‎src/client/pythonEnvironments/creation/common/createEnvTriggerUtils.ts‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { PVSC_EXTENSION_ID } from '../../../common/constants';
1010
import { PythonExtension } from '../../../api/types';
1111
import { traceVerbose } from '../../../logging';
1212
import { getConfiguration } from '../../../common/vscodeApis/workspaceApis';
13-
import { getWorkspaceStateValue, updateWorkspaceStateValue } from '../../../common/persistentState';
13+
import { getWorkspaceStateValue } from '../../../common/persistentState';
1414

1515
export const CREATE_ENV_TRIGGER_SETTING_PART = 'createEnvironment.trigger';
1616
export const CREATE_ENV_TRIGGER_SETTING = `python.${CREATE_ENV_TRIGGER_SETTING_PART}`;
@@ -90,15 +90,6 @@ export function disableCreateEnvironmentTrigger(): void {
9090
}
9191
}
9292

93-
/**
94-
* Sets trigger to 'off' in workspace persistent state. This disables trigger check
95-
* for the current workspace only. In multi root case, it is disabled for all folders
96-
* in the multi root workspace.
97-
*/
98-
export async function disableWorkspaceCreateEnvironmentTrigger(): Promise<void> {
99-
await updateWorkspaceStateValue(CREATE_ENV_TRIGGER_SETTING, 'off');
100-
}
101-
10293
let _alreadyCreateEnvCriteriaCheck = false;
10394
/**
10495
* Run-once wrapper function for the workspace check to prompt to create an environment.

‎src/client/pythonEnvironments/creation/createEnvironmentTrigger.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ import {
1010
shouldPromptToCreateEnv,
1111
isCreateEnvWorkspaceCheckNotRun,
1212
disableCreateEnvironmentTrigger,
13-
disableWorkspaceCreateEnvironmentTrigger,
1413
} from './common/createEnvTriggerUtils';
1514
import { getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis';
1615
import { traceError, traceInfo, traceVerbose } from '../../logging';
1716
import { hasPrefixCondaEnv, hasVenv } from './common/commonUtils';
1817
import { showInformationMessage } from '../../common/vscodeApis/windowApis';
19-
import { CreateEnv } from '../../common/utils/localize';
18+
import { Common, CreateEnv } from '../../common/utils/localize';
2019
import { executeCommand, registerCommand } from '../../common/vscodeApis/commandApis';
2120
import { Commands } from '../../common/constants';
2221
import { Resource } from '../../common/types';
@@ -77,8 +76,7 @@ async function createEnvironmentCheckForWorkspace(uri: Uri): Promise<void> {
7776
const selection = await showInformationMessage(
7877
CreateEnv.Trigger.workspaceTriggerMessage,
7978
CreateEnv.Trigger.createEnvironment,
80-
CreateEnv.Trigger.disableCheckWorkspace,
81-
CreateEnv.Trigger.disableCheck,
79+
Common.doNotShowAgain,
8280
);
8381

8482
if (selection === CreateEnv.Trigger.createEnvironment) {
@@ -87,10 +85,8 @@ async function createEnvironmentCheckForWorkspace(uri: Uri): Promise<void> {
8785
} catch (error) {
8886
traceError('CreateEnv Trigger - Error while creating environment: ', error);
8987
}
90-
} else if (selection === CreateEnv.Trigger.disableCheck) {
88+
} else if (selection === Common.doNotShowAgain) {
9189
disableCreateEnvironmentTrigger();
92-
} else if (selection === CreateEnv.Trigger.disableCheckWorkspace) {
93-
disableWorkspaceCreateEnvironmentTrigger();
9490
}
9591
}
9692

‎src/client/pythonEnvironments/creation/globalPipInTerminalTrigger.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ import { CreateEnvOnPipInstallTrigger } from '../../common/experiments/groups';
33
import { inExperiment } from '../common/externalDependencies';
44
import {
55
disableCreateEnvironmentTrigger,
6-
disableWorkspaceCreateEnvironmentTrigger,
76
isGlobalPythonSelected,
87
shouldPromptToCreateEnv,
98
} from './common/createEnvTriggerUtils';
109
import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/vscodeApis/workspaceApis';
11-
import { CreateEnv } from '../../common/utils/localize';
10+
import { Common, CreateEnv } from '../../common/utils/localize';
1211
import { traceError, traceInfo } from '../../logging';
1312
import { executeCommand } from '../../common/vscodeApis/commandApis';
1413
import { Commands, PVSC_EXTENSION_ID } from '../../common/constants';
@@ -46,8 +45,7 @@ export function registerTriggerForPipInTerminal(disposables: Disposable[]): void
4645
const selection = await showWarningMessage(
4746
CreateEnv.Trigger.globalPipInstallTriggerMessage,
4847
CreateEnv.Trigger.createEnvironment,
49-
CreateEnv.Trigger.disableCheckWorkspace,
50-
CreateEnv.Trigger.disableCheck,
48+
Common.doNotShowAgain,
5149
);
5250
if (selection === CreateEnv.Trigger.createEnvironment) {
5351
try {
@@ -69,10 +67,8 @@ export function registerTriggerForPipInTerminal(disposables: Disposable[]): void
6967
} catch (error) {
7068
traceError('CreateEnv Trigger - Error while creating environment: ', error);
7169
}
72-
} else if (selection === CreateEnv.Trigger.disableCheck) {
70+
} else if (selection === Common.doNotShowAgain) {
7371
disableCreateEnvironmentTrigger();
74-
} else if (selection === CreateEnv.Trigger.disableCheckWorkspace) {
75-
disableWorkspaceCreateEnvironmentTrigger();
7672
}
7773
}
7874
}

‎src/test/pythonEnvironments/creation/createEnvironmentTrigger.unit.test.ts‎

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis';
1616
import * as commandApis from '../../../client/common/vscodeApis/commandApis';
1717
import { Commands } from '../../../client/common/constants';
18-
import { CreateEnv } from '../../../client/common/utils/localize';
18+
import { Common, CreateEnv } from '../../../client/common/utils/localize';
1919

2020
suite('Create Environment Trigger', () => {
2121
let shouldPromptToCreateEnvStub: sinon.SinonStub;
@@ -29,7 +29,6 @@ suite('Create Environment Trigger', () => {
2929
let getWorkspaceFolderStub: sinon.SinonStub;
3030
let executeCommandStub: sinon.SinonStub;
3131
let disableCreateEnvironmentTriggerStub: sinon.SinonStub;
32-
let disableWorkspaceCreateEnvironmentTriggerStub: sinon.SinonStub;
3332

3433
const workspace1 = {
3534
uri: Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'workspace1')),
@@ -54,10 +53,6 @@ suite('Create Environment Trigger', () => {
5453

5554
executeCommandStub = sinon.stub(commandApis, 'executeCommand');
5655
disableCreateEnvironmentTriggerStub = sinon.stub(triggerUtils, 'disableCreateEnvironmentTrigger');
57-
disableWorkspaceCreateEnvironmentTriggerStub = sinon.stub(
58-
triggerUtils,
59-
'disableWorkspaceCreateEnvironmentTrigger',
60-
);
6156
});
6257

6358
teardown(() => {
@@ -208,7 +203,6 @@ suite('Create Environment Trigger', () => {
208203

209204
sinon.assert.notCalled(executeCommandStub);
210205
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
211-
sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
212206
});
213207

214208
test('Should show prompt if all conditions met: User clicks create', async () => {
@@ -232,18 +226,17 @@ suite('Create Environment Trigger', () => {
232226

233227
sinon.assert.calledOnceWithExactly(executeCommandStub, Commands.Create_Environment);
234228
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
235-
sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
236229
});
237230

238-
test('Should show prompt if all conditions met: User clicks disable global', async () => {
231+
test("Should show prompt if all conditions met: User clicks don't show again", async () => {
239232
shouldPromptToCreateEnvStub.returns(true);
240233
hasVenvStub.resolves(false);
241234
hasPrefixCondaEnvStub.resolves(false);
242235
hasRequirementFilesStub.resolves(true);
243236
hasKnownFilesStub.resolves(false);
244237
isGlobalPythonSelectedStub.resolves(true);
245238

246-
showInformationMessageStub.resolves(CreateEnv.Trigger.disableCheck);
239+
showInformationMessageStub.resolves(Common.doNotShowAgain);
247240
await triggerCreateEnvironmentCheck(CreateEnvironmentCheckKind.Workspace, workspace1.uri);
248241

249242
sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
@@ -256,30 +249,5 @@ suite('Create Environment Trigger', () => {
256249

257250
sinon.assert.notCalled(executeCommandStub);
258251
sinon.assert.calledOnce(disableCreateEnvironmentTriggerStub);
259-
sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
260-
});
261-
262-
test('Should show prompt if all conditions met: User clicks disable workspace', async () => {
263-
shouldPromptToCreateEnvStub.returns(true);
264-
hasVenvStub.resolves(false);
265-
hasPrefixCondaEnvStub.resolves(false);
266-
hasRequirementFilesStub.resolves(true);
267-
hasKnownFilesStub.resolves(false);
268-
isGlobalPythonSelectedStub.resolves(true);
269-
270-
showInformationMessageStub.resolves(CreateEnv.Trigger.disableCheckWorkspace);
271-
await triggerCreateEnvironmentCheck(CreateEnvironmentCheckKind.Workspace, workspace1.uri);
272-
273-
sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
274-
sinon.assert.calledOnce(hasVenvStub);
275-
sinon.assert.calledOnce(hasPrefixCondaEnvStub);
276-
sinon.assert.calledOnce(hasRequirementFilesStub);
277-
sinon.assert.calledOnce(hasKnownFilesStub);
278-
sinon.assert.calledOnce(isGlobalPythonSelectedStub);
279-
sinon.assert.calledOnce(showInformationMessageStub);
280-
281-
sinon.assert.notCalled(executeCommandStub);
282-
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
283-
sinon.assert.calledOnce(disableWorkspaceCreateEnvironmentTriggerStub);
284252
});
285253
});

‎src/test/pythonEnvironments/creation/globalPipInTerminalTrigger.unit.test.ts‎

Lines changed: 3 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import * as commandApis from '../../../client/common/vscodeApis/commandApis';
2020
import * as extDepApi from '../../../client/pythonEnvironments/common/externalDependencies';
2121
import { registerTriggerForPipInTerminal } from '../../../client/pythonEnvironments/creation/globalPipInTerminalTrigger';
2222
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants';
23-
import { CreateEnv } from '../../../client/common/utils/localize';
23+
import { Common, CreateEnv } from '../../../client/common/utils/localize';
2424

2525
suite('Global Pip in Terminal Trigger', () => {
2626
let shouldPromptToCreateEnvStub: sinon.SinonStub;
@@ -31,7 +31,6 @@ suite('Global Pip in Terminal Trigger', () => {
3131
let showWarningMessageStub: sinon.SinonStub;
3232
let executeCommandStub: sinon.SinonStub;
3333
let disableCreateEnvironmentTriggerStub: sinon.SinonStub;
34-
let disableWorkspaceCreateEnvironmentTriggerStub: sinon.SinonStub;
3534
let onDidStartTerminalShellExecutionStub: sinon.SinonStub;
3635
let handler: undefined | ((e: TerminalShellExecutionStartEvent) => Promise<void>);
3736
let execEvent: typemoq.IMock<TerminalShellExecutionStartEvent>;
@@ -64,10 +63,6 @@ suite('Global Pip in Terminal Trigger', () => {
6463
executeCommandStub.resolves({ path: 'some/python' });
6564

6665
disableCreateEnvironmentTriggerStub = sinon.stub(triggerUtils, 'disableCreateEnvironmentTrigger');
67-
disableWorkspaceCreateEnvironmentTriggerStub = sinon.stub(
68-
triggerUtils,
69-
'disableWorkspaceCreateEnvironmentTrigger',
70-
);
7166

7267
onDidStartTerminalShellExecutionStub = sinon.stub(windowApis, 'onDidStartTerminalShellExecution');
7368
onDidStartTerminalShellExecutionStub.callsFake((cb) => {
@@ -270,16 +265,15 @@ suite('Global Pip in Terminal Trigger', () => {
270265

271266
sinon.assert.calledOnce(executeCommandStub);
272267
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
273-
sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
274268

275269
shellIntegration.verify((s) => s.executeCommand(typemoq.It.isAnyString()), typemoq.Times.once());
276270
});
277271

278-
test('Should disable create environment trigger if user selects to disable', async () => {
272+
test("Should disable create environment trigger if user selects don't show again", async () => {
279273
shouldPromptToCreateEnvStub.returns(true);
280274
inExperimentStub.returns(true);
281275
isGlobalPythonSelectedStub.returns(true);
282-
showWarningMessageStub.resolves(CreateEnv.Trigger.disableCheck);
276+
showWarningMessageStub.resolves(Common.doNotShowAgain);
283277

284278
const disposables: Disposable[] = [];
285279
registerTriggerForPipInTerminal(disposables);
@@ -310,44 +304,5 @@ suite('Global Pip in Terminal Trigger', () => {
310304

311305
sinon.assert.notCalled(executeCommandStub);
312306
sinon.assert.calledOnce(disableCreateEnvironmentTriggerStub);
313-
sinon.assert.notCalled(disableWorkspaceCreateEnvironmentTriggerStub);
314-
});
315-
316-
test('Should disable workspace create environment trigger if user selects to disable', async () => {
317-
shouldPromptToCreateEnvStub.returns(true);
318-
inExperimentStub.returns(true);
319-
isGlobalPythonSelectedStub.returns(true);
320-
showWarningMessageStub.resolves(CreateEnv.Trigger.disableCheckWorkspace);
321-
322-
const disposables: Disposable[] = [];
323-
registerTriggerForPipInTerminal(disposables);
324-
325-
await handler?.({
326-
terminal: ({} as unknown) as Terminal,
327-
shellIntegration: shellIntegration.object,
328-
execution: {
329-
cwd: workspace1.uri,
330-
commandLine: {
331-
isTrusted: true,
332-
value: 'pip install',
333-
confidence: 0,
334-
},
335-
read: () =>
336-
(async function* () {
337-
yield Promise.resolve('pip install');
338-
})(),
339-
},
340-
});
341-
342-
assert.strictEqual(disposables.length, 1);
343-
sinon.assert.calledOnce(shouldPromptToCreateEnvStub);
344-
sinon.assert.calledOnce(inExperimentStub);
345-
sinon.assert.calledOnce(getWorkspaceFolderStub);
346-
sinon.assert.calledOnce(isGlobalPythonSelectedStub);
347-
sinon.assert.calledOnce(showWarningMessageStub);
348-
349-
sinon.assert.notCalled(executeCommandStub);
350-
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
351-
sinon.assert.calledOnce(disableWorkspaceCreateEnvironmentTriggerStub);
352307
});
353308
});

0 commit comments

Comments
 (0)