Skip to content

Commit

Permalink
Use sendText to send Python code to Terminal REPL for Python >= 3.13 (#…
Browse files Browse the repository at this point in the history
…24765)

Resolves:
#24674 (comment)
Use sendText to send Python code to Terminal REPL for Python >= 3.13 to
prevent keyboard interrupt.

Relevant file context from VS Code:
https://github.com/microsoft/vscode/blob/f9c927cf7a29a59b896b6cdac2d8b5d2d43afea5/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts#L906

It seems like we are on this edge scenario where generic terminal shell
integration is enabled (so executeCommand can be used), but we have
temporarily disabled Python shell integration for Python >= 3.13 (and of
course sending the relevant escape sequences such as the commandLine
itself to VS Code). Why?
* python/cpython#126131 placing user's mouse
cursor position at odd place.

Why and where I think the keyboard interrupt is happening:
Python extension tries to executeCommand when sending commands to
terminal REPL >= Python3.13, where we are not sending shell integration
escape sequences from the Python side.
* I think this is why it is attaching the keyboard interrupt all the
sudden, because VS Code see that Python extension is requesting
executeCommand but is not sending the commandLine escape sequence to
them. For every other versions < 3.13 (where we send all the shell
integration escape sequences including the commandLine), this does not
happen.
  • Loading branch information
anthonykim1 authored Feb 4, 2025
1 parent e9b4b7b commit 6b784e5
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 3 deletions.
11 changes: 10 additions & 1 deletion src/client/common/terminal/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { useEnvExtension } from '../../envExt/api.internal';
import { ensureTerminalLegacy } from '../../envExt/api.legacy';
import { sleep } from '../utils/async';
import { isWindows } from '../utils/platform';
import { getPythonMinorVersion } from '../../repl/replUtils';

@injectable()
export class TerminalService implements ITerminalService, Disposable {
Expand Down Expand Up @@ -108,7 +109,15 @@ export class TerminalService implements ITerminalService, Disposable {

const config = getConfiguration('python');
const pythonrcSetting = config.get<boolean>('terminal.shellIntegration.enabled');
if ((isPythonShell && !pythonrcSetting) || (isPythonShell && isWindows())) {

const minorVersion = this.options?.resource
? await getPythonMinorVersion(
this.options.resource,
this.serviceContainer.get<IInterpreterService>(IInterpreterService),
)
: undefined;

if ((isPythonShell && !pythonrcSetting) || (isPythonShell && isWindows()) || (minorVersion ?? 0) >= 13) {
// If user has explicitly disabled SI for Python, use sendText for inside Terminal REPL.
terminal.sendText(commandLine);
return undefined;
Expand Down
14 changes: 14 additions & 0 deletions src/client/repl/replUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,17 @@ export function getTabNameForUri(uri: Uri): string | undefined {

return undefined;
}

/**
* Function that will return the minor version of current active Python interpreter.
*/
export async function getPythonMinorVersion(
uri: Uri | undefined,
interpreterService: IInterpreterService,
): Promise<number | undefined> {
if (uri) {
const pythonVersion = await getActiveInterpreter(uri, interpreterService);
return pythonVersion?.version?.minor;
}
return undefined;
}
53 changes: 51 additions & 2 deletions src/test/common/terminals/service.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,29 @@ import {
TerminalShellExecution,
TerminalShellExecutionEndEvent,
TerminalShellIntegration,
Uri,
Terminal as VSCodeTerminal,
WorkspaceConfiguration,
} from 'vscode';
import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types';
import { EXTENSION_ROOT_DIR } from '../../../client/common/constants';
import { IPlatformService } from '../../../client/common/platform/types';
import { TerminalService } from '../../../client/common/terminal/service';
import { ITerminalActivator, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types';
import {
ITerminalActivator,
ITerminalHelper,
TerminalCreationOptions,
TerminalShellType,
} from '../../../client/common/terminal/types';
import { IDisposableRegistry } from '../../../client/common/types';
import { IServiceContainer } from '../../../client/ioc/types';
import { ITerminalAutoActivation } from '../../../client/terminals/types';
import { createPythonInterpreter } from '../../utils/interpreters';
import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis';
import * as platform from '../../../client/common/utils/platform';
import * as extapi from '../../../client/envExt/api.internal';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';

suite('Terminal Service', () => {
let service: TerminalService;
Expand All @@ -46,6 +54,8 @@ suite('Terminal Service', () => {
let editorConfig: TypeMoq.IMock<WorkspaceConfiguration>;
let isWindowsStub: sinon.SinonStub;
let useEnvExtensionStub: sinon.SinonStub;
let interpreterService: TypeMoq.IMock<IInterpreterService>;
let options: TypeMoq.IMock<TerminalCreationOptions>;

setup(() => {
useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension');
Expand Down Expand Up @@ -92,6 +102,13 @@ suite('Terminal Service', () => {
disposables = [];

mockServiceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
interpreterService
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));

options = TypeMoq.Mock.ofType<TerminalCreationOptions>();
options.setup((o) => o.resource).returns(() => Uri.parse('a'));

mockServiceContainer.setup((c) => c.get(ITerminalManager)).returns(() => terminalManager.object);
mockServiceContainer.setup((c) => c.get(ITerminalHelper)).returns(() => terminalHelper.object);
Expand All @@ -100,6 +117,7 @@ suite('Terminal Service', () => {
mockServiceContainer.setup((c) => c.get(IWorkspaceService)).returns(() => workspaceService.object);
mockServiceContainer.setup((c) => c.get(ITerminalActivator)).returns(() => terminalActivator.object);
mockServiceContainer.setup((c) => c.get(ITerminalAutoActivation)).returns(() => terminalAutoActivator.object);
mockServiceContainer.setup((c) => c.get(IInterpreterService)).returns(() => interpreterService.object);
getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
isWindowsStub = sinon.stub(platform, 'isWindows');
pythonConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
Expand All @@ -117,6 +135,7 @@ suite('Terminal Service', () => {
}
disposables.filter((item) => !!item).forEach((item) => item.dispose());
sinon.restore();
interpreterService.reset();
});

test('Ensure terminal is disposed', async () => {
Expand Down Expand Up @@ -239,7 +258,7 @@ suite('Terminal Service', () => {
terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.exactly(1));
});

test('Ensure sendText is NOT called when Python shell integration and terminal shell integration are both enabled - Mac, Linux', async () => {
test('Ensure sendText is NOT called when Python shell integration and terminal shell integration are both enabled - Mac, Linux && Python < 3.13', async () => {
isWindowsStub.returns(false);
pythonConfig
.setup((p) => p.get('terminal.shellIntegration.enabled'))
Expand All @@ -261,6 +280,36 @@ suite('Terminal Service', () => {
terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.never());
});

test('Ensure sendText is called when Python shell integration and terminal shell integration are both enabled - Mac, Linux && Python >= 3.13', async () => {
interpreterService.reset();

interpreterService
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
.returns(() =>
Promise.resolve({ path: 'yo', version: { major: 3, minor: 13, patch: 0 } } as PythonEnvironment),
);

isWindowsStub.returns(false);
pythonConfig
.setup((p) => p.get('terminal.shellIntegration.enabled'))
.returns(() => true)
.verifiable(TypeMoq.Times.once());

terminalHelper
.setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns(() => Promise.resolve(undefined));

service = new TerminalService(mockServiceContainer.object, options.object);
const textToSend = 'Some Text';
terminalHelper.setup((h) => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash);
terminalManager.setup((t) => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object);

await service.ensureTerminal();
await service.executeCommand(textToSend, true);

terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.once());
});

test('Ensure sendText IS called even when Python shell integration and terminal shell integration are both enabled - Window', async () => {
isWindowsStub.returns(true);
pythonConfig
Expand Down

0 comments on commit 6b784e5

Please sign in to comment.