Skip to content
Open
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
7 changes: 6 additions & 1 deletion src/features/terminal/terminalActivationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { PythonEnvironment } from '../../api';
import { traceError, traceInfo, traceVerbose } from '../../common/logging';
import { onDidEndTerminalShellExecution, onDidStartTerminalShellExecution } from '../../common/window.apis';
import { getActivationCommand, getDeactivationCommand } from '../common/activation';
import { getShellIntegrationTimeout, isTaskTerminal } from './utils';
import { getShellIntegrationTimeout, isTaskTerminal, shouldSkipTerminalActivation } from './utils';

export interface DidChangeTerminalActivationStateEvent {
terminal: Terminal;
Expand Down Expand Up @@ -86,6 +86,11 @@ export class TerminalActivationImpl implements TerminalActivationInternal {
}

async activate(terminal: Terminal, environment: PythonEnvironment): Promise<void> {
if (shouldSkipTerminalActivation(terminal)) {
traceVerbose('Skipping activation for this terminal');
return;
}

if (isTaskTerminal(terminal)) {
traceVerbose('Cannot activate environment in a task terminal');
return;
Expand Down
25 changes: 16 additions & 9 deletions src/features/terminal/terminalManager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fsapi from 'fs-extra';
import * as path from 'path';
import { Disposable, EventEmitter, ProgressLocation, Terminal, TerminalOptions, Uri } from 'vscode';
import { Disposable, EventEmitter, ProgressLocation, Terminal, Uri } from 'vscode';
import { PythonEnvironment, PythonEnvironmentApi, PythonProject, PythonTerminalCreateOptions } from '../../api';
import { ActivationStrings } from '../../common/localize';
import { traceInfo, traceVerbose } from '../../common/logging';
Expand Down Expand Up @@ -34,6 +34,7 @@ import {
getAutoActivationType,
getEnvironmentForTerminal,
shouldActivateInCurrentTerminal,
shouldSkipTerminalActivation,
waitForShellIntegration,
} from './utils';

Expand Down Expand Up @@ -94,7 +95,7 @@ export class TerminalManagerImpl implements TerminalManager {
this.onTerminalClosedEmitter.fire(t);
}),
this.onTerminalOpened(async (t) => {
if (this.skipActivationOnOpen.has(t) || (t.creationOptions as TerminalOptions)?.hideFromUser) {
if (this.skipActivationOnOpen.has(t) || shouldSkipTerminalActivation(t)) {
return;
}
let env = this.ta.getEnvironment(t);
Expand Down Expand Up @@ -419,7 +420,11 @@ export class TerminalManagerImpl implements TerminalManager {

if (actType === ACT_TYPE_COMMAND) {
if (!skipPreExistingTerminals) {
await Promise.all(terminals().map(async (t) => this.activateUsingCommand(api, t)));
await Promise.all(
terminals()
.filter((t) => !shouldSkipTerminalActivation(t))
.map(async (t) => this.activateUsingCommand(api, t)),
);
}
} else if (actType === ACT_TYPE_SHELL) {
const shells = new Set(
Expand All @@ -431,12 +436,14 @@ export class TerminalManagerImpl implements TerminalManager {
await this.handleSetupCheck(shells);
if (!skipPreExistingTerminals) {
await Promise.all(
terminals().map(async (t) => {
// If the shell is not set up, we activate using command fallback.
if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
await this.activateUsingCommand(api, t);
}
}),
terminals()
.filter((t) => !shouldSkipTerminalActivation(t))
.map(async (t) => {
// If the shell is not set up, we activate using command fallback.
if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
await this.activateUsingCommand(api, t);
}
}),
);
}
}
Expand Down
15 changes: 14 additions & 1 deletion src/features/terminal/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as path from 'path';
import { Disposable, env, tasks, Terminal, TerminalOptions, Uri } from 'vscode';
import { Disposable, env, ExtensionTerminalOptions, tasks, Terminal, TerminalOptions, Uri } from 'vscode';
import { PythonEnvironment, PythonProject, PythonProjectEnvironmentApi, PythonProjectGetterApi } from '../../api';
import { traceVerbose } from '../../common/logging';
import { timeout } from '../../common/utils/asyncUtils';
Expand Down Expand Up @@ -385,3 +385,16 @@ export function removeAnsiEscapeCodes(str: string): string {

return str;
}

/**
* Determines if a terminal should be skipped for environment activation based on its creation options.
* Terminals that are hidden from the user or are PTY-based extension terminals are skipped.
* @param terminal The terminal to check.
* @returns `true` if the terminal should be skipped; `false` otherwise.
*/
export function shouldSkipTerminalActivation(terminal: Terminal): boolean {
return (
!!(terminal.creationOptions as TerminalOptions)?.hideFromUser ||
!!(terminal.creationOptions as ExtensionTerminalOptions)?.pty
);
}
147 changes: 147 additions & 0 deletions src/test/features/terminal/terminalManager.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Disposable,
Event,
EventEmitter,
ExtensionTerminalOptions,
Progress,
Terminal,
TerminalOptions,
Expand Down Expand Up @@ -617,3 +618,149 @@ suite('TerminalManager - initialize() with activateEnvInCurrentTerminal', () =>
);
});
});

suite('TerminalManager - initialize() skips pseudoterminals', () => {
let terminalActivation: TestTerminalActivation;
let terminalManager: TerminalManagerImpl;
let mockGetAutoActivationType: sinon.SinonStub;
let mockShouldActivateInCurrentTerminal: sinon.SinonStub;
let mockTerminals: sinon.SinonStub;
let mockGetEnvironmentForTerminal: sinon.SinonStub;

const createMockTerminal = (name: string, options?: TerminalOptions | ExtensionTerminalOptions): Terminal =>
({
name,
creationOptions: options ?? ({} as TerminalOptions),
shellIntegration: undefined,
show: sinon.stub(),
sendText: sinon.stub(),
}) as unknown as Terminal;

const createMockEnvironment = (): PythonEnvironment => ({
envId: { id: 'test-env-id', managerId: 'test-manager' },
name: 'Test Environment',
displayName: 'Test Environment',
shortDisplayName: 'TestEnv',
displayPath: '/path/to/env',
version: '3.9.0',
environmentPath: Uri.file('/path/to/python'),
sysPrefix: '/path/to/env',
execInfo: {
run: { executable: '/path/to/python' },
activation: [{ executable: '/path/to/activate' }],
},
});

setup(() => {
terminalActivation = new TestTerminalActivation();

mockGetAutoActivationType = sinon.stub(terminalUtils, 'getAutoActivationType');
mockShouldActivateInCurrentTerminal = sinon.stub(terminalUtils, 'shouldActivateInCurrentTerminal');
mockGetEnvironmentForTerminal = sinon.stub(terminalUtils, 'getEnvironmentForTerminal');
sinon.stub(terminalUtils, 'waitForShellIntegration').resolves(false);
sinon.stub(activationUtils, 'isActivatableEnvironment').returns(true);
sinon.stub(shellDetector, 'identifyTerminalShell').returns('bash');

sinon.stub(windowApis, 'createTerminal').callsFake(() => createMockTerminal('new'));
sinon.stub(windowApis, 'onDidOpenTerminal').returns(new Disposable(() => {}));
sinon.stub(windowApis, 'onDidCloseTerminal').returns(new Disposable(() => {}));
sinon.stub(windowApis, 'onDidChangeWindowState').returns(new Disposable(() => {}));
sinon.stub(windowApis, 'activeTerminal').returns(undefined);

mockTerminals = sinon.stub(windowApis, 'terminals');

sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
const mockProgress = { report: () => {} };
const mockToken = {
isCancellationRequested: false,
onCancellationRequested: () => new Disposable(() => {}),
};
return task(mockProgress as never, mockToken as never);
});
sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => {}));
});

teardown(() => {
sinon.restore();
terminalActivation.dispose();
});

test('initialize skips pseudoterminals and activates regular terminals (ACT_TYPE_COMMAND)', async () => {
const regularTerminal = createMockTerminal('regular');
const pseudoTerminal = createMockTerminal('pseudo', {
name: 'pseudo',
pty: { open: () => {}, close: () => {} },
} as unknown as ExtensionTerminalOptions);
const env = createMockEnvironment();

mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
mockShouldActivateInCurrentTerminal.returns(true);
mockTerminals.returns([regularTerminal, pseudoTerminal]);
mockGetEnvironmentForTerminal.resolves(env);

terminalManager = new TerminalManagerImpl(terminalActivation, [], []);
await terminalManager.initialize({} as never);

assert.strictEqual(
terminalActivation.activateCalls,
1,
'Should only activate the regular terminal, not the pseudoterminal',
);
});

test('initialize skips pseudoterminals in shell startup fallback path (ACT_TYPE_SHELL)', async () => {
const regularTerminal = createMockTerminal('regular');
const pseudoTerminal = createMockTerminal('pseudo', {
name: 'pseudo',
pty: { open: () => {}, close: () => {} },
} as unknown as ExtensionTerminalOptions);
const env = createMockEnvironment();

const mockShellProvider: ShellStartupScriptProvider = {
name: 'bash-test',
shellType: 'bash',
isSetup: sinon.stub().resolves(ShellSetupState.NotSetup),
setupScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
teardownScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
clearCache: sinon.stub().resolves(),
};
sinon.stub(shellUtils, 'getShellIntegrationEnabledCache').resolves(false);
sinon.stub(shellUtils, 'shouldUseProfileActivation').returns(false);

mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_SHELL);
mockShouldActivateInCurrentTerminal.returns(true);
mockTerminals.returns([regularTerminal, pseudoTerminal]);
mockGetEnvironmentForTerminal.resolves(env);

terminalManager = new TerminalManagerImpl(terminalActivation, [], [mockShellProvider]);
await terminalManager.initialize({} as never);

assert.strictEqual(
terminalActivation.activateCalls,
1,
'Should only activate the regular terminal via command fallback, not the pseudoterminal',
);
});

test('initialize skips all terminals when only pseudoterminals exist (ACT_TYPE_COMMAND)', async () => {
const pseudo1 = createMockTerminal('pseudo1', {
name: 'pseudo1',
pty: { open: () => {}, close: () => {} },
} as unknown as ExtensionTerminalOptions);
const pseudo2 = createMockTerminal('pseudo2', {
name: 'pseudo2',
pty: { open: () => {}, close: () => {} },
} as unknown as ExtensionTerminalOptions);
const env = createMockEnvironment();

mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
mockShouldActivateInCurrentTerminal.returns(true);
mockTerminals.returns([pseudo1, pseudo2]);
mockGetEnvironmentForTerminal.resolves(env);

terminalManager = new TerminalManagerImpl(terminalActivation, [], []);
await terminalManager.initialize({} as never);

assert.strictEqual(terminalActivation.activateCalls, 0, 'Should not activate any pseudoterminals');
});
});
42 changes: 41 additions & 1 deletion src/test/features/terminal/utils.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as assert from 'assert';
import * as sinon from 'sinon';
import { Terminal } from 'vscode';
import { ExtensionTerminalOptions, Terminal, TerminalOptions } from 'vscode';
import * as windowApis from '../../../common/window.apis';
import * as workspaceApis from '../../../common/workspace.apis';
import * as shellDetector from '../../../features/common/shellDetector';
Expand All @@ -11,6 +11,7 @@ import {
AutoActivationType,
getAutoActivationType,
shouldActivateInCurrentTerminal,
shouldSkipTerminalActivation,
waitForShellIntegration,
} from '../../../features/terminal/utils';

Expand Down Expand Up @@ -550,6 +551,45 @@ suite('Terminal Utils - shouldActivateInCurrentTerminal', () => {
});
});

suite('Terminal Utils - shouldSkipTerminalActivation', () => {
test('should return false for a regular terminal with no special options', () => {
const terminal = { creationOptions: {} as TerminalOptions } as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
});

test('should return true when hideFromUser is true', () => {
const terminal = { creationOptions: { hideFromUser: true } as TerminalOptions } as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
});

test('should return false when hideFromUser is false', () => {
const terminal = { creationOptions: { hideFromUser: false } as TerminalOptions } as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
});

test('should return true for a pseudoterminal (pty-based extension terminal)', () => {
const terminal = {
creationOptions: {
name: 'pseudo',
pty: { open: () => {}, close: () => {} },
} as unknown as ExtensionTerminalOptions,
} as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
});

test('should return false when pty is undefined', () => {
const terminal = { creationOptions: {} as ExtensionTerminalOptions } as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
});

test('should return true when both hideFromUser and pty are set', () => {
const terminal = {
creationOptions: { hideFromUser: true, pty: { open: () => {}, close: () => {} } },
} as unknown as Terminal;
assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
});
});

suite('Terminal Utils - waitForShellIntegration', () => {
let mockGetConfiguration: sinon.SinonStub;
let identifyTerminalShellStub: sinon.SinonStub;
Expand Down
Loading