Skip to content

Commit 147bf2f

Browse files
anthonykim1anthonykim1
committed
Respect activateInCurrentEnv
1 parent 3fda1e6 commit 147bf2f

File tree

4 files changed

+308
-9
lines changed

4 files changed

+308
-9
lines changed

‎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+
// Match vscode-python behavior: 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: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,39 @@ 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 the currently focused terminal and previous terminal.
267+
* Checks the legacy `python.terminal.activateEnvInCurrentTerminal` setting.
268+
*
269+
* - If the user has explicitly set the value to `false` at any scope
270+
* (global, workspace, or workspace folder), returns `false`.
271+
* - Otherwise (default or explicitly `true`), returns `true`.
272+
*
273+
* @returns `false` only when the user has explicitly set the setting to `false`; `true` otherwise.
274+
*/
275+
export function shouldActivateInCurrentTerminal(): boolean {
276+
const pythonConfig = getConfiguration('python');
277+
const inspected = pythonConfig.inspect<boolean>('terminal.activateEnvInCurrentTerminal');
278+
279+
if (!inspected) {
280+
return true;
281+
}
282+
283+
// Check explicit user-set values at each scope (highest precedence first).
284+
// Only respect `false` when the user has deliberately set it.
285+
if (inspected.workspaceFolderValue === false) {
286+
return false;
287+
}
288+
if (inspected.workspaceValue === false) {
289+
return false;
290+
}
291+
if (inspected.globalValue === false) {
292+
return false;
293+
}
294+
295+
return true;
296+
}
297+
265298
export async function getAllDistinctProjectEnvironments(
266299
api: PythonProjectGetterApi & PythonProjectEnvironmentApi,
267300
): Promise<PythonEnvironment[] | undefined> {

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,3 +421,129 @@ suite('TerminalManager - terminal naming', () => {
421421
}
422422
});
423423
});
424+
425+
suite('TerminalManager - initialize() with activateEnvInCurrentTerminal', () => {
426+
let terminalActivation: TestTerminalActivation;
427+
let terminalManager: TerminalManagerImpl;
428+
let mockGetAutoActivationType: sinon.SinonStub;
429+
let mockShouldActivateInCurrentTerminal: sinon.SinonStub;
430+
let mockTerminals: sinon.SinonStub;
431+
let mockGetEnvironmentForTerminal: sinon.SinonStub;
432+
433+
const createMockTerminal = (name: string): Terminal =>
434+
({
435+
name,
436+
creationOptions: {} as TerminalOptions,
437+
shellIntegration: undefined,
438+
show: sinon.stub(),
439+
sendText: sinon.stub(),
440+
}) as unknown as Terminal;
441+
442+
const createMockEnvironment = (): PythonEnvironment => ({
443+
envId: { id: 'test-env-id', managerId: 'test-manager' },
444+
name: 'Test Environment',
445+
displayName: 'Test Environment',
446+
shortDisplayName: 'TestEnv',
447+
displayPath: '/path/to/env',
448+
version: '3.9.0',
449+
environmentPath: Uri.file('/path/to/python'),
450+
sysPrefix: '/path/to/env',
451+
execInfo: {
452+
run: { executable: '/path/to/python' },
453+
activation: [{ executable: '/path/to/activate' }],
454+
},
455+
});
456+
457+
setup(() => {
458+
terminalActivation = new TestTerminalActivation();
459+
460+
mockGetAutoActivationType = sinon.stub(terminalUtils, 'getAutoActivationType');
461+
mockShouldActivateInCurrentTerminal = sinon.stub(terminalUtils, 'shouldActivateInCurrentTerminal');
462+
mockGetEnvironmentForTerminal = sinon.stub(terminalUtils, 'getEnvironmentForTerminal');
463+
sinon.stub(terminalUtils, 'waitForShellIntegration').resolves(false);
464+
sinon.stub(activationUtils, 'isActivatableEnvironment').returns(true);
465+
sinon.stub(shellDetector, 'identifyTerminalShell').returns('bash');
466+
467+
sinon.stub(windowApis, 'createTerminal').callsFake(() => createMockTerminal('new'));
468+
sinon.stub(windowApis, 'onDidOpenTerminal').returns(new Disposable(() => {}));
469+
sinon.stub(windowApis, 'onDidCloseTerminal').returns(new Disposable(() => {}));
470+
sinon.stub(windowApis, 'onDidChangeWindowState').returns(new Disposable(() => {}));
471+
sinon.stub(windowApis, 'activeTerminal').returns(undefined);
472+
473+
mockTerminals = sinon.stub(windowApis, 'terminals');
474+
475+
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
476+
const mockProgress = { report: () => {} };
477+
const mockToken = {
478+
isCancellationRequested: false,
479+
onCancellationRequested: () => new Disposable(() => {}),
480+
};
481+
return task(mockProgress as never, mockToken as never);
482+
});
483+
sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => {}));
484+
});
485+
486+
teardown(() => {
487+
sinon.restore();
488+
terminalActivation.dispose();
489+
});
490+
491+
function createTerminalManager(): TerminalManagerImpl {
492+
return new TerminalManagerImpl(terminalActivation, [], []);
493+
}
494+
495+
test('initialize activates all pre-existing terminals when shouldActivateInCurrentTerminal returns true', async () => {
496+
const terminal1 = createMockTerminal('terminal1');
497+
const terminal2 = createMockTerminal('terminal2');
498+
const env = createMockEnvironment();
499+
500+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
501+
mockShouldActivateInCurrentTerminal.returns(true);
502+
mockTerminals.returns([terminal1, terminal2]);
503+
mockGetEnvironmentForTerminal.resolves(env);
504+
505+
terminalManager = createTerminalManager();
506+
await terminalManager.initialize({} as never);
507+
508+
assert.strictEqual(
509+
terminalActivation.activateCalls,
510+
2,
511+
'Should activate all pre-existing terminals when activateEnvInCurrentTerminal is true/default',
512+
);
513+
});
514+
515+
test('initialize skips all pre-existing terminals when shouldActivateInCurrentTerminal returns false', async () => {
516+
const terminal1 = createMockTerminal('terminal1');
517+
const terminal2 = createMockTerminal('terminal2');
518+
const env = createMockEnvironment();
519+
520+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
521+
mockShouldActivateInCurrentTerminal.returns(false);
522+
mockTerminals.returns([terminal1, terminal2]);
523+
mockGetEnvironmentForTerminal.resolves(env);
524+
525+
terminalManager = createTerminalManager();
526+
await terminalManager.initialize({} as never);
527+
528+
assert.strictEqual(
529+
terminalActivation.activateCalls,
530+
0,
531+
'Should skip all pre-existing terminals when activateEnvInCurrentTerminal is explicitly false',
532+
);
533+
});
534+
535+
test('initialize proceeds normally when shouldActivateInCurrentTerminal returns false but no pre-existing terminals', async () => {
536+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
537+
mockShouldActivateInCurrentTerminal.returns(false);
538+
mockTerminals.returns([]);
539+
540+
terminalManager = createTerminalManager();
541+
await terminalManager.initialize({} as never);
542+
543+
assert.strictEqual(
544+
terminalActivation.activateCalls,
545+
0,
546+
'Should have no activations when there are no terminals',
547+
);
548+
});
549+
});

‎src/test/features/terminal/utils.unit.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
ACT_TYPE_SHELL,
88
AutoActivationType,
99
getAutoActivationType,
10+
shouldActivateInCurrentTerminal,
1011
} from '../../../features/terminal/utils';
1112

1213
interface MockWorkspaceConfig {
@@ -354,3 +355,126 @@ suite('Terminal Utils - getAutoActivationType', () => {
354355
});
355356
});
356357
});
358+
359+
suite('Terminal Utils - shouldActivateInCurrentTerminal', () => {
360+
let mockGetConfiguration: sinon.SinonStub;
361+
let pythonConfig: MockWorkspaceConfig;
362+
363+
setup(() => {
364+
mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration');
365+
366+
pythonConfig = {
367+
get: sinon.stub(),
368+
inspect: sinon.stub(),
369+
update: sinon.stub(),
370+
};
371+
372+
mockGetConfiguration.withArgs('python').returns(pythonConfig);
373+
});
374+
375+
teardown(() => {
376+
sinon.restore();
377+
});
378+
379+
test('should return true when inspect returns undefined (no config)', () => {
380+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns(undefined);
381+
382+
assert.strictEqual(shouldActivateInCurrentTerminal(), true, 'Should default to true when no config exists');
383+
});
384+
385+
test('should return true when no explicit values are set (all undefined)', () => {
386+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
387+
key: 'terminal.activateEnvInCurrentTerminal',
388+
defaultValue: false,
389+
globalValue: undefined,
390+
workspaceValue: undefined,
391+
workspaceFolderValue: undefined,
392+
});
393+
394+
assert.strictEqual(
395+
shouldActivateInCurrentTerminal(),
396+
true,
397+
'Should return true when only defaultValue is set (not user-explicit)',
398+
);
399+
});
400+
401+
test('should return false when globalValue is explicitly false', () => {
402+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
403+
key: 'terminal.activateEnvInCurrentTerminal',
404+
defaultValue: false,
405+
globalValue: false,
406+
workspaceValue: undefined,
407+
workspaceFolderValue: undefined,
408+
});
409+
410+
assert.strictEqual(
411+
shouldActivateInCurrentTerminal(),
412+
false,
413+
'Should return false when user explicitly set globalValue to false',
414+
);
415+
});
416+
417+
test('should return false when workspaceValue is explicitly false', () => {
418+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
419+
key: 'terminal.activateEnvInCurrentTerminal',
420+
defaultValue: false,
421+
globalValue: undefined,
422+
workspaceValue: false,
423+
workspaceFolderValue: undefined,
424+
});
425+
426+
assert.strictEqual(
427+
shouldActivateInCurrentTerminal(),
428+
false,
429+
'Should return false when user explicitly set workspaceValue to false',
430+
);
431+
});
432+
433+
test('should return false when workspaceFolderValue is explicitly false', () => {
434+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
435+
key: 'terminal.activateEnvInCurrentTerminal',
436+
defaultValue: false,
437+
globalValue: undefined,
438+
workspaceValue: undefined,
439+
workspaceFolderValue: false,
440+
});
441+
442+
assert.strictEqual(
443+
shouldActivateInCurrentTerminal(),
444+
false,
445+
'Should return false when user explicitly set workspaceFolderValue to false',
446+
);
447+
});
448+
449+
test('should return true when globalValue is explicitly true', () => {
450+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
451+
key: 'terminal.activateEnvInCurrentTerminal',
452+
defaultValue: false,
453+
globalValue: true,
454+
workspaceValue: undefined,
455+
workspaceFolderValue: undefined,
456+
});
457+
458+
assert.strictEqual(
459+
shouldActivateInCurrentTerminal(),
460+
true,
461+
'Should return true when user explicitly set globalValue to true',
462+
);
463+
});
464+
465+
test('workspaceFolderValue false takes precedence over globalValue true', () => {
466+
pythonConfig.inspect.withArgs('terminal.activateEnvInCurrentTerminal').returns({
467+
key: 'terminal.activateEnvInCurrentTerminal',
468+
defaultValue: false,
469+
globalValue: true,
470+
workspaceValue: undefined,
471+
workspaceFolderValue: false,
472+
});
473+
474+
assert.strictEqual(
475+
shouldActivateInCurrentTerminal(),
476+
false,
477+
'workspaceFolderValue false should take precedence',
478+
);
479+
});
480+
});

0 commit comments

Comments
 (0)