Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/client/common/terminal/activator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down
37 changes: 36 additions & 1 deletion src/client/envExt/api.internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>('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<boolean>('useEnvironmentsExtension');
if (folderInspection?.workspaceFolderValue === false) {
return false;
}
}
}
return true;
}

let _useExt: boolean | undefined;
export function useEnvExtension(): boolean {
if (_useExt !== undefined) {
Expand Down
4 changes: 2 additions & 2 deletions src/client/providers/terminalProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand All @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/client/terminals/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/common/terminals/activation.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -25,6 +27,7 @@ suite('Terminal Auto Activation', () => {
let terminal: Terminal;

setup(() => {
sinon.stub(extapi, 'shouldEnvExtHandleActivation').returns(false);
terminal = ({
dispose: noop,
hide: noop,
Expand All @@ -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;
Expand Down
95 changes: 94 additions & 1 deletion src/test/common/terminals/activator/index.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -29,9 +31,12 @@ suite('Terminal Activator', () => {
let terminalSettings: TypeMoq.IMock<ITerminalSettings>;
let experimentService: TypeMoq.IMock<IExperimentService>;
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<ITerminalActivator>();
terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
Expand Down Expand Up @@ -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<Terminal>();
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);
});
});
3 changes: 3 additions & 0 deletions src/test/providers/terminal.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ suite('Terminal Provider', () => {
let experimentService: TypeMoq.IMock<IExperimentService>;
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<IServiceContainer>();
commandManager = TypeMoq.Mock.ofType<ICommandManager>();
Expand Down
16 changes: 16 additions & 0 deletions src/test/terminals/activation.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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', () => {
Expand All @@ -21,8 +23,12 @@ suite('Terminal', () => {
let onDidOpenTerminalEventEmitter: EventEmitter<Terminal>;
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);
Expand Down Expand Up @@ -60,6 +66,9 @@ suite('Terminal', () => {
autoActivation.register();
});
// teardown(() => fakeTimer.uninstall());
teardown(() => {
sinon.restore();
});

test('Should activate terminal', async () => {
// Trigger opening a terminal.
Expand All @@ -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<void>);

verify(activator.activateEnvironmentInTerminal(anything(), anything())).never();
});
});
});
Loading