diff --git a/src/client/common/terminal/activator/index.ts b/src/client/common/terminal/activator/index.ts index 24ffb5008364..cde04bdbf10d 100644 --- a/src/client/common/terminal/activator/index.ts +++ b/src/client/common/terminal/activator/index.ts @@ -9,7 +9,7 @@ import { IConfigurationService, IExperimentService } from '../../types'; import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types'; import { BaseTerminalActivator } from './base'; import { inTerminalEnvVarExperiment } from '../../experiments/helpers'; -import { useEnvExtension } from '../../../envExt/api.internal'; +import { shouldEnvExtHandleActivation } from '../../../envExt/api.internal'; import { EventName } from '../../../telemetry/constants'; import { sendTelemetryEvent } from '../../../telemetry'; @@ -44,8 +44,8 @@ export class TerminalActivator implements ITerminalActivator { const settings = this.configurationService.getSettings(options?.resource); const activateEnvironment = settings.terminal.activateEnvironment && !inTerminalEnvVarExperiment(this.experimentService); - if (!activateEnvironment || options?.hideFromUser || useEnvExtension()) { - if (useEnvExtension()) { + if (!activateEnvironment || options?.hideFromUser || shouldEnvExtHandleActivation()) { + if (shouldEnvExtHandleActivation()) { sendTelemetryEvent(EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL); } return false; diff --git a/src/client/envExt/api.internal.ts b/src/client/envExt/api.internal.ts index 5acdd5bba8e3..5edfb712072e 100644 --- a/src/client/envExt/api.internal.ts +++ b/src/client/envExt/api.internal.ts @@ -13,12 +13,47 @@ import { DidChangeEnvironmentEventArgs, } from './types'; import { executeCommand } from '../common/vscodeApis/commandApis'; -import { getConfiguration } from '../common/vscodeApis/workspaceApis'; +import { getConfiguration, getWorkspaceFolders } from '../common/vscodeApis/workspaceApis'; import { traceError, traceLog } from '../logging'; import { Interpreters } from '../common/utils/localize'; export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs'; +export function isEnvExtensionInstalled(): boolean { + return !!getExtension(ENVS_EXTENSION_ID); +} + +/** + * Returns true if the Python Environments extension is installed and not explicitly + * disabled by the user. Mirrors the envs extension's own activation logic: it + * deactivates only when `python.useEnvironmentsExtension` is explicitly set to false + * at the global, workspace, or workspace-folder level. + */ +export function shouldEnvExtHandleActivation(): boolean { + if (!isEnvExtensionInstalled()) { + return false; + } + const config = getConfiguration('python'); + const inspection = config.inspect('useEnvironmentsExtension'); + if (inspection?.globalValue === false || inspection?.workspaceValue === false) { + return false; + } + // The envs extension also checks folder-scoped settings in multi-root workspaces. + // Any single folder with the setting set to false causes the envs extension to + // deactivate entirely (window-wide), so we must mirror that here. + const workspaceFolders = getWorkspaceFolders(); + if (workspaceFolders) { + for (const folder of workspaceFolders) { + const folderConfig = getConfiguration('python', folder.uri); + const folderInspection = folderConfig.inspect('useEnvironmentsExtension'); + if (folderInspection?.workspaceFolderValue === false) { + return false; + } + } + } + return true; +} + let _useExt: boolean | undefined; export function useEnvExtension(): boolean { if (_useExt !== undefined) { diff --git a/src/client/providers/terminalProvider.ts b/src/client/providers/terminalProvider.ts index 841f479269ac..f68f151110ec 100644 --- a/src/client/providers/terminalProvider.ts +++ b/src/client/providers/terminalProvider.ts @@ -11,7 +11,7 @@ import { swallowExceptions } from '../common/utils/decorators'; import { IServiceContainer } from '../ioc/types'; import { captureTelemetry, sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; -import { useEnvExtension } from '../envExt/api.internal'; +import { useEnvExtension, shouldEnvExtHandleActivation } from '../envExt/api.internal'; export class TerminalProvider implements Disposable { private disposables: Disposable[] = []; @@ -33,7 +33,7 @@ export class TerminalProvider implements Disposable { currentTerminal && pythonSettings.terminal.activateEnvInCurrentTerminal && !inTerminalEnvVarExperiment(experimentService) && - !useEnvExtension() + !shouldEnvExtHandleActivation() ) { const hideFromUser = 'hideFromUser' in currentTerminal.creationOptions && currentTerminal.creationOptions.hideFromUser; diff --git a/src/client/terminals/activation.ts b/src/client/terminals/activation.ts index 143a2de14e5c..ed26916e3eaa 100644 --- a/src/client/terminals/activation.ts +++ b/src/client/terminals/activation.ts @@ -9,6 +9,7 @@ import { IActiveResourceService, ITerminalManager } from '../common/application/ import { ITerminalActivator } from '../common/terminal/types'; import { IDisposable, IDisposableRegistry } from '../common/types'; import { ITerminalAutoActivation } from './types'; +import { shouldEnvExtHandleActivation } from '../envExt/api.internal'; @injectable() export class TerminalAutoActivation implements ITerminalAutoActivation { @@ -49,6 +50,9 @@ export class TerminalAutoActivation implements ITerminalAutoActivation { if (this.terminalsNotToAutoActivate.has(terminal)) { return; } + if (shouldEnvExtHandleActivation()) { + return; + } if ('hideFromUser' in terminal.creationOptions && terminal.creationOptions.hideFromUser) { return; } diff --git a/src/test/common/terminals/activation.unit.test.ts b/src/test/common/terminals/activation.unit.test.ts index 49ada1c06b11..d87d33ea03e6 100644 --- a/src/test/common/terminals/activation.unit.test.ts +++ b/src/test/common/terminals/activation.unit.test.ts @@ -3,6 +3,7 @@ 'use strict'; import { expect } from 'chai'; +import * as sinon from 'sinon'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Terminal, Uri } from 'vscode'; @@ -15,6 +16,7 @@ import { IDisposable } from '../../../client/common/types'; import { TerminalAutoActivation } from '../../../client/terminals/activation'; import { ITerminalAutoActivation } from '../../../client/terminals/types'; import { noop } from '../../core'; +import * as extapi from '../../../client/envExt/api.internal'; suite('Terminal Auto Activation', () => { let activator: ITerminalActivator; @@ -25,6 +27,7 @@ suite('Terminal Auto Activation', () => { let terminal: Terminal; setup(() => { + sinon.stub(extapi, 'shouldEnvExtHandleActivation').returns(false); terminal = ({ dispose: noop, hide: noop, @@ -46,6 +49,9 @@ suite('Terminal Auto Activation', () => { instance(activeResourceService), ); }); + teardown(() => { + sinon.restore(); + }); test('New Terminals should be activated', async () => { type EventHandler = (e: Terminal) => void; diff --git a/src/test/common/terminals/activator/index.unit.test.ts b/src/test/common/terminals/activator/index.unit.test.ts index 6a50901bc99d..34d1cf8f1bcd 100644 --- a/src/test/common/terminals/activator/index.unit.test.ts +++ b/src/test/common/terminals/activator/index.unit.test.ts @@ -6,7 +6,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; -import { Terminal } from 'vscode'; +import { Terminal, Uri } from 'vscode'; import { TerminalActivator } from '../../../../client/common/terminal/activator'; import { ITerminalActivationHandler, @@ -20,6 +20,8 @@ import { ITerminalSettings, } from '../../../../client/common/types'; import * as extapi from '../../../../client/envExt/api.internal'; +import * as workspaceApis from '../../../../client/common/vscodeApis/workspaceApis'; +import * as extensionsApi from '../../../../client/common/vscodeApis/extensionsApi'; suite('Terminal Activator', () => { let activator: TerminalActivator; @@ -29,9 +31,12 @@ suite('Terminal Activator', () => { let terminalSettings: TypeMoq.IMock; let experimentService: TypeMoq.IMock; let useEnvExtensionStub: sinon.SinonStub; + let shouldEnvExtHandleActivationStub: sinon.SinonStub; setup(() => { useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension'); useEnvExtensionStub.returns(false); + shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation'); + shouldEnvExtHandleActivationStub.returns(false); baseActivator = TypeMoq.Mock.ofType(); terminalSettings = TypeMoq.Mock.ofType(); @@ -113,4 +118,92 @@ suite('Terminal Activator', () => { test('Terminal is not activated if auto-activate setting is set to true but terminal is hidden', () => testActivationAndHandlers(false, true, true)); test('Terminal is not activated and handlers are invoked', () => testActivationAndHandlers(false, false)); + + test('Terminal is not activated from Python extension when Env extension should handle activation', async () => { + shouldEnvExtHandleActivationStub.returns(true); + terminalSettings.setup((b) => b.activateEnvironment).returns(() => true); + baseActivator + .setup((b) => b.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.never()); + + const terminal = TypeMoq.Mock.ofType(); + const activated = await activator.activateEnvironmentInTerminal(terminal.object, { + preserveFocus: true, + }); + + assert.strictEqual(activated, false); + baseActivator.verifyAll(); + }); +}); + +suite('shouldEnvExtHandleActivation', () => { + let getExtensionStub: sinon.SinonStub; + let getConfigurationStub: sinon.SinonStub; + let getWorkspaceFoldersStub: sinon.SinonStub; + + setup(() => { + getExtensionStub = sinon.stub(extensionsApi, 'getExtension'); + getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration'); + getWorkspaceFoldersStub = sinon.stub(workspaceApis, 'getWorkspaceFolders'); + getWorkspaceFoldersStub.returns(undefined); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Returns false when envs extension is not installed', () => { + getExtensionStub.returns(undefined); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false); + }); + + test('Returns true when envs extension is installed and setting is not explicitly set', () => { + getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID }); + getConfigurationStub.returns({ + inspect: () => ({ globalValue: undefined, workspaceValue: undefined }), + }); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), true); + }); + + test('Returns false when envs extension is installed but globalValue is false', () => { + getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID }); + getConfigurationStub.returns({ + inspect: () => ({ globalValue: false, workspaceValue: undefined }), + }); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false); + }); + + test('Returns false when envs extension is installed but workspaceValue is false', () => { + getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID }); + getConfigurationStub.returns({ + inspect: () => ({ globalValue: undefined, workspaceValue: false }), + }); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false); + }); + + test('Returns true when envs extension is installed and setting is explicitly true', () => { + getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID }); + getConfigurationStub.returns({ + inspect: () => ({ globalValue: true, workspaceValue: undefined }), + }); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), true); + }); + + test('Returns false when a workspace folder has workspaceFolderValue set to false', () => { + getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID }); + const folderUri = Uri.parse('file:///workspace/folder1'); + getWorkspaceFoldersStub.returns([{ uri: folderUri, name: 'folder1', index: 0 }]); + getConfigurationStub.callsFake((_section: string, scope?: Uri) => { + if (scope) { + return { + inspect: () => ({ workspaceFolderValue: false }), + }; + } + return { + inspect: () => ({ globalValue: undefined, workspaceValue: undefined }), + }; + }); + assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false); + }); }); diff --git a/src/test/providers/terminal.unit.test.ts b/src/test/providers/terminal.unit.test.ts index ac39ded922c8..8f684835b7cf 100644 --- a/src/test/providers/terminal.unit.test.ts +++ b/src/test/providers/terminal.unit.test.ts @@ -29,10 +29,13 @@ suite('Terminal Provider', () => { let experimentService: TypeMoq.IMock; let terminalProvider: TerminalProvider; let useEnvExtensionStub: sinon.SinonStub; + let shouldEnvExtHandleActivationStub: sinon.SinonStub; const resource = Uri.parse('a'); setup(() => { useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension'); useEnvExtensionStub.returns(false); + shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation'); + shouldEnvExtHandleActivationStub.returns(false); serviceContainer = TypeMoq.Mock.ofType(); commandManager = TypeMoq.Mock.ofType(); diff --git a/src/test/terminals/activation.unit.test.ts b/src/test/terminals/activation.unit.test.ts index dea0c891229d..4c5294a82f49 100644 --- a/src/test/terminals/activation.unit.test.ts +++ b/src/test/terminals/activation.unit.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { anything, instance, mock, verify, when } from 'ts-mockito'; +import * as sinon from 'sinon'; import { EventEmitter, Terminal } from 'vscode'; import { ActiveResourceService } from '../../client/common/application/activeResource'; import { TerminalManager } from '../../client/common/application/terminalManager'; @@ -11,6 +12,7 @@ import { ITerminalActivator } from '../../client/common/terminal/types'; import { TerminalAutoActivation } from '../../client/terminals/activation'; import { ITerminalAutoActivation } from '../../client/terminals/types'; import { noop } from '../core'; +import * as extapi from '../../client/envExt/api.internal'; suite('Terminal', () => { suite('Terminal Auto Activation', () => { @@ -21,8 +23,12 @@ suite('Terminal', () => { let onDidOpenTerminalEventEmitter: EventEmitter; let terminal: Terminal; let nonActivatedTerminal: Terminal; + let shouldEnvExtHandleActivationStub: sinon.SinonStub; setup(() => { + shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation'); + shouldEnvExtHandleActivationStub.returns(false); + manager = mock(TerminalManager); activator = mock(TerminalActivator); resourceService = mock(ActiveResourceService); @@ -60,6 +66,9 @@ suite('Terminal', () => { autoActivation.register(); }); // teardown(() => fakeTimer.uninstall()); + teardown(() => { + sinon.restore(); + }); test('Should activate terminal', async () => { // Trigger opening a terminal. @@ -77,5 +86,12 @@ suite('Terminal', () => { // The terminal should get activated. verify(activator.activateEnvironmentInTerminal(anything(), anything())).never(); }); + test('Should not activate terminal when envs extension should handle activation', async () => { + shouldEnvExtHandleActivationStub.returns(true); + + await ((onDidOpenTerminalEventEmitter.fire(terminal) as unknown) as Promise); + + verify(activator.activateEnvironmentInTerminal(anything(), anything())).never(); + }); }); });