Skip to content

Commit 05b9e23

Browse files
authored
Better handle activateEnvInCurrentTerminal from Python extension (#1318)
Resolves: #1300 Behaviors after this PR: Note that the default false and explicit true behaves the same. https://github.com/user-attachments/assets/f90fc9cb-edf9-4d72-a6ea-66df6c11aad2 https://github.com/user-attachments/assets/afa3e541-3272-48e0-9146-97efe5ed6782
1 parent cad3dc5 commit 05b9e23

4 files changed

Lines changed: 459 additions & 10 deletions

File tree

src/features/terminal/terminalManager.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
AutoActivationType,
3434
getAutoActivationType,
3535
getEnvironmentForTerminal,
36+
shouldActivateInCurrentTerminal,
3637
waitForShellIntegration,
3738
} from './utils';
3839

@@ -405,8 +406,21 @@ export class TerminalManagerImpl implements TerminalManager {
405406

406407
public async initialize(api: PythonEnvironmentApi): Promise<void> {
407408
const actType = getAutoActivationType();
409+
410+
// When activateEnvInCurrentTerminal is explicitly false,
411+
// skip activation for ALL pre-existing terminals (terminals open before extension load).
412+
// New terminals opened after extension load are still activated via autoActivateOnTerminalOpen.
413+
const skipPreExistingTerminals = !shouldActivateInCurrentTerminal() && terminals().length > 0;
414+
if (skipPreExistingTerminals) {
415+
traceVerbose(
416+
'python.terminal.activateEnvInCurrentTerminal is explicitly disabled, skipping activation for pre-existing terminals',
417+
);
418+
}
419+
408420
if (actType === ACT_TYPE_COMMAND) {
409-
await Promise.all(terminals().map(async (t) => this.activateUsingCommand(api, t)));
421+
if (!skipPreExistingTerminals) {
422+
await Promise.all(terminals().map(async (t) => this.activateUsingCommand(api, t)));
423+
}
410424
} else if (actType === ACT_TYPE_SHELL) {
411425
const shells = new Set(
412426
terminals()
@@ -415,14 +429,16 @@ export class TerminalManagerImpl implements TerminalManager {
415429
);
416430
if (shells.size > 0) {
417431
await this.handleSetupCheck(shells);
418-
await Promise.all(
419-
terminals().map(async (t) => {
420-
// If the shell is not set up, we activate using command fallback.
421-
if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
422-
await this.activateUsingCommand(api, t);
423-
}
424-
}),
425-
);
432+
if (!skipPreExistingTerminals) {
433+
await Promise.all(
434+
terminals().map(async (t) => {
435+
// If the shell is not set up, we activate using command fallback.
436+
if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
437+
await this.activateUsingCommand(api, t);
438+
}
439+
}),
440+
);
441+
}
426442
}
427443
}
428444
}

src/features/terminal/utils.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,52 @@ export async function setAutoActivationType(value: AutoActivationType): Promise<
262262
return await config.update('terminal.autoActivationType', value, true);
263263
}
264264

265+
/**
266+
* Determines whether activation commands should be sent to pre-existing terminals
267+
* (terminals open before extension load).
268+
*
269+
* Checks the legacy `python.terminal.activateEnvInCurrentTerminal` setting using `inspect()`
270+
* to distinguish between the default value and an explicitly user-set value.
271+
*
272+
* Priority: workspaceFolderValue > workspaceValue > globalRemoteValue > globalLocalValue > globalValue
273+
* (matches the precedence used by getShellIntegrationEnabledCache and getAutoActivationType)
274+
*
275+
* - If the user has explicitly set the value to `false` at any scope, returns `false`.
276+
* - Otherwise (default or explicitly `true`), returns `true`.
277+
*
278+
* @returns `false` only when the user has explicitly set the setting to `false`; `true` otherwise.
279+
*/
280+
export function shouldActivateInCurrentTerminal(): boolean {
281+
const pythonConfig = getConfiguration('python');
282+
const inspected = pythonConfig.inspect<boolean>('terminal.activateEnvInCurrentTerminal');
283+
284+
if (!inspected) {
285+
return true;
286+
}
287+
288+
// Only respect `false` when the user has deliberately set it.
289+
// Priority: workspaceFolder > workspace > globalRemote > globalLocal > global
290+
const inspectValue = inspected as Record<string, unknown>;
291+
292+
if (inspected.workspaceFolderValue === false) {
293+
return false;
294+
}
295+
if (inspected.workspaceValue === false) {
296+
return false;
297+
}
298+
if ('globalRemoteValue' in inspected && inspectValue.globalRemoteValue === false) {
299+
return false;
300+
}
301+
if ('globalLocalValue' in inspected && inspectValue.globalLocalValue === false) {
302+
return false;
303+
}
304+
if (inspected.globalValue === false) {
305+
return false;
306+
}
307+
308+
return true;
309+
}
310+
265311
export async function getAllDistinctProjectEnvironments(
266312
api: PythonProjectGetterApi & PythonProjectEnvironmentApi,
267313
): Promise<PythonEnvironment[] | undefined> {

src/test/features/terminal/terminalManager.unit.test.ts

Lines changed: 197 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ import * as windowApis from '../../../common/window.apis';
2121
import * as workspaceApis from '../../../common/workspace.apis';
2222
import * as activationUtils from '../../../features/common/activation';
2323
import * as shellDetector from '../../../features/common/shellDetector';
24-
import { ShellEnvsProvider, ShellStartupScriptProvider } from '../../../features/terminal/shells/startupProvider';
24+
import * as shellUtils from '../../../features/terminal/shells/common/shellUtils';
25+
import {
26+
ShellEnvsProvider,
27+
ShellScriptEditState,
28+
ShellSetupState,
29+
ShellStartupScriptProvider,
30+
} from '../../../features/terminal/shells/startupProvider';
2531
import {
2632
DidChangeTerminalActivationStateEvent,
2733
TerminalActivationInternal,
@@ -421,3 +427,193 @@ suite('TerminalManager - terminal naming', () => {
421427
}
422428
});
423429
});
430+
431+
suite('TerminalManager - initialize() with activateEnvInCurrentTerminal', () => {
432+
let terminalActivation: TestTerminalActivation;
433+
let terminalManager: TerminalManagerImpl;
434+
let mockGetAutoActivationType: sinon.SinonStub;
435+
let mockShouldActivateInCurrentTerminal: sinon.SinonStub;
436+
let mockTerminals: sinon.SinonStub;
437+
let mockGetEnvironmentForTerminal: sinon.SinonStub;
438+
439+
const createMockTerminal = (name: string): Terminal =>
440+
({
441+
name,
442+
creationOptions: {} as TerminalOptions,
443+
shellIntegration: undefined,
444+
show: sinon.stub(),
445+
sendText: sinon.stub(),
446+
}) as unknown as Terminal;
447+
448+
const createMockEnvironment = (): PythonEnvironment => ({
449+
envId: { id: 'test-env-id', managerId: 'test-manager' },
450+
name: 'Test Environment',
451+
displayName: 'Test Environment',
452+
shortDisplayName: 'TestEnv',
453+
displayPath: '/path/to/env',
454+
version: '3.9.0',
455+
environmentPath: Uri.file('/path/to/python'),
456+
sysPrefix: '/path/to/env',
457+
execInfo: {
458+
run: { executable: '/path/to/python' },
459+
activation: [{ executable: '/path/to/activate' }],
460+
},
461+
});
462+
463+
setup(() => {
464+
terminalActivation = new TestTerminalActivation();
465+
466+
mockGetAutoActivationType = sinon.stub(terminalUtils, 'getAutoActivationType');
467+
mockShouldActivateInCurrentTerminal = sinon.stub(terminalUtils, 'shouldActivateInCurrentTerminal');
468+
mockGetEnvironmentForTerminal = sinon.stub(terminalUtils, 'getEnvironmentForTerminal');
469+
sinon.stub(terminalUtils, 'waitForShellIntegration').resolves(false);
470+
sinon.stub(activationUtils, 'isActivatableEnvironment').returns(true);
471+
sinon.stub(shellDetector, 'identifyTerminalShell').returns('bash');
472+
473+
sinon.stub(windowApis, 'createTerminal').callsFake(() => createMockTerminal('new'));
474+
sinon.stub(windowApis, 'onDidOpenTerminal').returns(new Disposable(() => {}));
475+
sinon.stub(windowApis, 'onDidCloseTerminal').returns(new Disposable(() => {}));
476+
sinon.stub(windowApis, 'onDidChangeWindowState').returns(new Disposable(() => {}));
477+
sinon.stub(windowApis, 'activeTerminal').returns(undefined);
478+
479+
mockTerminals = sinon.stub(windowApis, 'terminals');
480+
481+
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
482+
const mockProgress = { report: () => {} };
483+
const mockToken = {
484+
isCancellationRequested: false,
485+
onCancellationRequested: () => new Disposable(() => {}),
486+
};
487+
return task(mockProgress as never, mockToken as never);
488+
});
489+
sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => {}));
490+
});
491+
492+
teardown(() => {
493+
sinon.restore();
494+
terminalActivation.dispose();
495+
});
496+
497+
function createTerminalManager(): TerminalManagerImpl {
498+
return new TerminalManagerImpl(terminalActivation, [], []);
499+
}
500+
501+
test('initialize activates all pre-existing terminals when shouldActivateInCurrentTerminal returns true', async () => {
502+
const terminal1 = createMockTerminal('terminal1');
503+
const terminal2 = createMockTerminal('terminal2');
504+
const env = createMockEnvironment();
505+
506+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
507+
mockShouldActivateInCurrentTerminal.returns(true);
508+
mockTerminals.returns([terminal1, terminal2]);
509+
mockGetEnvironmentForTerminal.resolves(env);
510+
511+
terminalManager = createTerminalManager();
512+
await terminalManager.initialize({} as never);
513+
514+
assert.strictEqual(
515+
terminalActivation.activateCalls,
516+
2,
517+
'Should activate all pre-existing terminals when activateEnvInCurrentTerminal is true/default',
518+
);
519+
});
520+
521+
test('initialize skips all pre-existing terminals when shouldActivateInCurrentTerminal returns false', async () => {
522+
const terminal1 = createMockTerminal('terminal1');
523+
const terminal2 = createMockTerminal('terminal2');
524+
const env = createMockEnvironment();
525+
526+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
527+
mockShouldActivateInCurrentTerminal.returns(false);
528+
mockTerminals.returns([terminal1, terminal2]);
529+
mockGetEnvironmentForTerminal.resolves(env);
530+
531+
terminalManager = createTerminalManager();
532+
await terminalManager.initialize({} as never);
533+
534+
assert.strictEqual(
535+
terminalActivation.activateCalls,
536+
0,
537+
'Should skip all pre-existing terminals when activateEnvInCurrentTerminal is explicitly false',
538+
);
539+
});
540+
541+
test('initialize proceeds normally when shouldActivateInCurrentTerminal returns false but no pre-existing terminals', async () => {
542+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
543+
mockShouldActivateInCurrentTerminal.returns(false);
544+
mockTerminals.returns([]);
545+
546+
terminalManager = createTerminalManager();
547+
await terminalManager.initialize({} as never);
548+
549+
assert.strictEqual(
550+
terminalActivation.activateCalls,
551+
0,
552+
'Should have no activations when there are no terminals',
553+
);
554+
});
555+
556+
test('initialize skips shell fallback activation for pre-existing terminals when shouldActivateInCurrentTerminal returns false (ACT_TYPE_SHELL)', async () => {
557+
const terminal1 = createMockTerminal('terminal1');
558+
const env = createMockEnvironment();
559+
560+
// Mock a shell startup provider that reports NotSetup so shellSetup gets set to false,
561+
// which would normally trigger the command fallback activation.
562+
const mockShellProvider: ShellStartupScriptProvider = {
563+
name: 'bash-test',
564+
shellType: 'bash',
565+
isSetup: sinon.stub().resolves(ShellSetupState.NotSetup),
566+
setupScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
567+
teardownScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
568+
clearCache: sinon.stub().resolves(),
569+
};
570+
sinon.stub(shellUtils, 'getShellIntegrationEnabledCache').resolves(false);
571+
sinon.stub(shellUtils, 'shouldUseProfileActivation').returns(false);
572+
573+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_SHELL);
574+
mockShouldActivateInCurrentTerminal.returns(false);
575+
mockTerminals.returns([terminal1]);
576+
mockGetEnvironmentForTerminal.resolves(env);
577+
578+
terminalManager = new TerminalManagerImpl(terminalActivation, [], [mockShellProvider]);
579+
await terminalManager.initialize({} as never);
580+
581+
assert.strictEqual(
582+
terminalActivation.activateCalls,
583+
0,
584+
'Should skip shell fallback activation for pre-existing terminals when activateEnvInCurrentTerminal is explicitly false',
585+
);
586+
});
587+
588+
test('initialize activates via shell command fallback for pre-existing terminals when shouldActivateInCurrentTerminal returns true (ACT_TYPE_SHELL)', async () => {
589+
const terminal1 = createMockTerminal('terminal1');
590+
const env = createMockEnvironment();
591+
592+
// Mock a shell startup provider that reports NotSetup so shellSetup gets set to false,
593+
// triggering the command fallback activation.
594+
const mockShellProvider: ShellStartupScriptProvider = {
595+
name: 'bash-test',
596+
shellType: 'bash',
597+
isSetup: sinon.stub().resolves(ShellSetupState.NotSetup),
598+
setupScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
599+
teardownScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
600+
clearCache: sinon.stub().resolves(),
601+
};
602+
sinon.stub(shellUtils, 'getShellIntegrationEnabledCache').resolves(false);
603+
sinon.stub(shellUtils, 'shouldUseProfileActivation').returns(false);
604+
605+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_SHELL);
606+
mockShouldActivateInCurrentTerminal.returns(true);
607+
mockTerminals.returns([terminal1]);
608+
mockGetEnvironmentForTerminal.resolves(env);
609+
610+
terminalManager = new TerminalManagerImpl(terminalActivation, [], [mockShellProvider]);
611+
await terminalManager.initialize({} as never);
612+
613+
assert.strictEqual(
614+
terminalActivation.activateCalls,
615+
1,
616+
'Should activate via command fallback when shell setup reports not setup',
617+
);
618+
});
619+
});

0 commit comments

Comments
 (0)