Skip to content

Commit

Permalink
Fix) Prevent keyboard interrupt for Python3.13 REPL non-Windows (#24555)
Browse files Browse the repository at this point in the history
Further resolves:
#24422
Prevent keyboard interrupt for Mac and Linux when using Python3.13

Having Python3.13 as interpreter choice and then enabling shell
integration where it is normally supported (we disabled temporarily for
Python3.13 due to python/cpython#126131), lead
to edge case.

So although we don't override user's PS1 in Python side after checking
Python3.13 is selected, we were not aware of this in typescript side,
leading to wrongly using executeCommand inside Python terminal REPL
(Python3.13 IDLE), instead of sendText.
  • Loading branch information
anthonykim1 authored Dec 9, 2024
1 parent b8cc93d commit faf37e2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
14 changes: 12 additions & 2 deletions src/client/common/terminal/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
import { traceVerbose } from '../../logging';
import { getConfiguration } from '../vscodeApis/workspaceApis';
import { isWindows } from '../utils/platform';
import { getActiveInterpreter } from '../../repl/replUtils';

@injectable()
export class TerminalService implements ITerminalService, Disposable {
Expand Down Expand Up @@ -102,10 +103,19 @@ export class TerminalService implements ITerminalService, Disposable {
});
await promise;
}

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

let isPython313 = false;
if (this.options && this.options.resource) {
const pythonVersion = await getActiveInterpreter(
this.options.resource,
this.serviceContainer.get<IInterpreterService>(IInterpreterService),
);
pythonVersion?.sysVersion?.startsWith('3.13');
}

if (isPythonShell && (!pythonrcSetting || isWindows() || isPython313)) {
// If user has explicitly disabled SI for Python, use sendText for inside Terminal REPL.
terminal.sendText(commandLine);
return undefined;
Expand Down
11 changes: 10 additions & 1 deletion src/test/common/terminals/service.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ 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 { IInterpreterService } from '../../../client/interpreter/contracts';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';

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

setup(() => {
terminal = TypeMoq.Mock.ofType<VSCodeTerminal>();
Expand Down Expand Up @@ -87,6 +90,10 @@ 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));

mockServiceContainer.setup((c) => c.get(ITerminalManager)).returns(() => terminalManager.object);
mockServiceContainer.setup((c) => c.get(ITerminalHelper)).returns(() => terminalHelper.object);
Expand All @@ -95,6 +102,8 @@ 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 Down Expand Up @@ -234,7 +243,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 - !Python3.13', async () => {
isWindowsStub.returns(false);
pythonConfig
.setup((p) => p.get('terminal.shellIntegration.enabled'))
Expand Down

0 comments on commit faf37e2

Please sign in to comment.