Skip to content

Commit

Permalink
Add more command patterns for pip trigger (#23273)
Browse files Browse the repository at this point in the history
  • Loading branch information
karthiknadig authored Apr 22, 2024
1 parent 51cf852 commit 26d5132
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ import { onDidStartTerminalShellExecution, showWarningMessage } from '../../comm
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';

function checkCommand(command: string): boolean {
const lower = command.toLowerCase();
return (
lower.startsWith('pip install') ||
lower.startsWith('pip3 install') ||
lower.startsWith('python -m pip install') ||
lower.startsWith('python3 -m pip install')
);
}

export function registerTriggerForPipInTerminal(disposables: Disposable[]): void {
if (!shouldPromptToCreateEnv() || !inExperiment(CreateEnvOnPipInstallTrigger.experiment)) {
return;
Expand All @@ -39,7 +49,7 @@ export function registerTriggerForPipInTerminal(disposables: Disposable[]): void
!createEnvironmentTriggered.get(workspaceFolder.uri.fsPath) &&
(await isGlobalPythonSelected(workspaceFolder))
) {
if (e.execution.commandLine.isTrusted && e.execution.commandLine.value.startsWith('pip install')) {
if (e.execution.commandLine.isTrusted && checkCommand(e.execution.commandLine.value)) {
createEnvironmentTriggered.set(workspaceFolder.uri.fsPath, true);
sendTelemetryEvent(EventName.ENVIRONMENT_TERMINAL_GLOBAL_PIP);
const selection = await showWarningMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,43 +230,45 @@ suite('Global Pip in Terminal Trigger', () => {
sinon.assert.notCalled(showWarningMessageStub);
});

test('Should prompt to create environment if all conditions are met', async () => {
shouldPromptToCreateEnvStub.returns(true);
inExperimentStub.returns(true);
isGlobalPythonSelectedStub.returns(true);
showWarningMessageStub.resolves(CreateEnv.Trigger.createEnvironment);

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,
['pip install', 'pip3 install', 'python -m pip install', 'python3 -m pip install'].forEach((command) => {
test(`Should prompt to create environment if all conditions are met: ${command}`, async () => {
shouldPromptToCreateEnvStub.returns(true);
inExperimentStub.returns(true);
isGlobalPythonSelectedStub.returns(true);
showWarningMessageStub.resolves(CreateEnv.Trigger.createEnvironment);

const disposables: Disposable[] = [];
registerTriggerForPipInTerminal(disposables);

await handler?.({
terminal: ({} as unknown) as Terminal,
shellIntegration: shellIntegration.object,
execution: {
cwd: workspace1.uri,
commandLine: {
isTrusted: true,
value: command,
confidence: 0,
},
read: () =>
(async function* () {
yield Promise.resolve(command);
})(),
},
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);
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.calledOnce(executeCommandStub);
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);
sinon.assert.calledOnce(executeCommandStub);
sinon.assert.notCalled(disableCreateEnvironmentTriggerStub);

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

test("Should disable create environment trigger if user selects don't show again", async () => {
Expand Down

0 comments on commit 26d5132

Please sign in to comment.