From 6b784e53e70a30c0895cafd60c8ad76b2b087749 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Tue, 4 Feb 2025 11:39:42 -0800 Subject: [PATCH] Use sendText to send Python code to Terminal REPL for Python >= 3.13 (#24765) Resolves: https://github.com/microsoft/vscode-python/issues/24674#issuecomment-2574108023 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? * https://github.com/python/cpython/issues/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. --- src/client/common/terminal/service.ts | 11 +++- src/client/repl/replUtils.ts | 14 +++++ .../common/terminals/service.unit.test.ts | 53 ++++++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index b02670836015..a051d66f015f 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -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 { @@ -108,7 +109,15 @@ export class TerminalService implements ITerminalService, Disposable { const config = getConfiguration('python'); const pythonrcSetting = config.get('terminal.shellIntegration.enabled'); - if ((isPythonShell && !pythonrcSetting) || (isPythonShell && isWindows())) { + + const minorVersion = this.options?.resource + ? await getPythonMinorVersion( + this.options.resource, + this.serviceContainer.get(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; diff --git a/src/client/repl/replUtils.ts b/src/client/repl/replUtils.ts index 0c2c4ba0d84e..8e23218c2870 100644 --- a/src/client/repl/replUtils.ts +++ b/src/client/repl/replUtils.ts @@ -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 { + if (uri) { + const pythonVersion = await getActiveInterpreter(uri, interpreterService); + return pythonVersion?.version?.minor; + } + return undefined; +} diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 9903f6781f28..63a1cd544940 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -11,6 +11,7 @@ import { TerminalShellExecution, TerminalShellExecutionEndEvent, TerminalShellIntegration, + Uri, Terminal as VSCodeTerminal, WorkspaceConfiguration, } from 'vscode'; @@ -18,7 +19,12 @@ import { ITerminalManager, IWorkspaceService } from '../../../client/common/appl 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'; @@ -26,6 +32,8 @@ 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; @@ -46,6 +54,8 @@ suite('Terminal Service', () => { let editorConfig: TypeMoq.IMock; let isWindowsStub: sinon.SinonStub; let useEnvExtensionStub: sinon.SinonStub; + let interpreterService: TypeMoq.IMock; + let options: TypeMoq.IMock; setup(() => { useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension'); @@ -92,6 +102,13 @@ suite('Terminal Service', () => { disposables = []; mockServiceContainer = TypeMoq.Mock.ofType(); + interpreterService = TypeMoq.Mock.ofType(); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + + options = TypeMoq.Mock.ofType(); + 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); @@ -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(); @@ -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 () => { @@ -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')) @@ -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