From c7a150471c62de9e6fab0914b34cd0a3b6c72309 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 11 Nov 2024 10:12:57 -0800 Subject: [PATCH 1/2] Start on partial keyboard interrupt fixes Co-authored-by: Daniel Imms --- src/client/common/terminal/service.ts | 34 +++++++++++++++---- .../common/terminal/syncTerminalService.ts | 8 ++--- src/client/common/terminal/types.ts | 9 +++-- .../codeExecution/terminalCodeExecution.ts | 13 +++++-- src/client/terminals/types.ts | 1 + 5 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 45ce9afac47e..f20ad8496bdf 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -2,17 +2,18 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { CancellationToken, Disposable, Event, EventEmitter, Terminal, TerminalShellExecution } from 'vscode'; +import { CancellationToken, Disposable, Event, EventEmitter, Terminal, window } from 'vscode'; import '../../common/extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { ITerminalAutoActivation } from '../../terminals/types'; +import { ICodeExecutionService, ITerminalAutoActivation } from '../../terminals/types'; import { ITerminalManager } from '../application/types'; import { _SCRIPTS_DIR } from '../process/internal/scripts/constants'; import { IConfigurationService, IDisposableRegistry } from '../types'; import { + IExecuteCommandResult, ITerminalActivator, ITerminalHelper, ITerminalService, @@ -57,14 +58,18 @@ export class TerminalService implements ITerminalService, Disposable { }); } } - public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise { + public async sendCommand( + command: string, + args: string[], + _?: CancellationToken, + ): Promise { await this.ensureTerminal(); const text = this.terminalHelper.buildCommandForTerminal(this.terminalShellType, command, args); if (!this.options?.hideFromUser) { this.terminal!.show(true); } - await this.executeCommand(text); + return this.executeCommand(text); } /** @deprecated */ public async sendText(text: string): Promise { @@ -74,7 +79,7 @@ export class TerminalService implements ITerminalService, Disposable { } this.terminal!.sendText(text); } - public async executeCommand(commandLine: string): Promise { + public async executeCommand(commandLine: string): Promise { const terminal = this.terminal!; if (!this.options?.hideFromUser) { terminal.show(true); @@ -97,11 +102,26 @@ export class TerminalService implements ITerminalService, Disposable { }); await promise; } - + // python in a shell , exit code is undefined . startCommand event happen, we call end command event if (terminal.shellIntegration) { + // TODO: Await the python REPL execute promise here. So we know python repl launched for sure before executing other python code. + // So we would not be interrupted. + + await this.serviceContainer.get(ICodeExecutionService).replActive; + const execution = terminal.shellIntegration.executeCommand(commandLine); traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`); - return execution; + return { + execution, + exitCode: new Promise((resolve) => { + const disposable = window.onDidEndTerminalShellExecution((event) => { + if (event.execution === execution) { + disposable.dispose(); + resolve(event.exitCode); + } + }); + }), + }; } else { terminal.sendText(commandLine); traceVerbose(`Shell Integration is disabled, sendText: ${commandLine}`); diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts index 60f8ed7a6847..265635cf18b6 100644 --- a/src/client/common/terminal/syncTerminalService.ts +++ b/src/client/common/terminal/syncTerminalService.ts @@ -4,7 +4,7 @@ 'use strict'; import { inject } from 'inversify'; -import { CancellationToken, Disposable, Event, TerminalShellExecution } from 'vscode'; +import { CancellationToken, Disposable, Event } from 'vscode'; import { IInterpreterService } from '../../interpreter/contracts'; import { traceVerbose } from '../../logging'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts'; import { createDeferred, Deferred } from '../utils/async'; import { noop } from '../utils/misc'; import { TerminalService } from './service'; -import { ITerminalService } from './types'; +import { IExecuteCommandResult, ITerminalService } from './types'; enum State { notStarted = 0, @@ -124,7 +124,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable args: string[], cancel?: CancellationToken, swallowExceptions: boolean = true, - ): Promise { + ): Promise { if (!cancel) { return this.terminalService.sendCommand(command, args); } @@ -145,7 +145,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable public sendText(text: string): Promise { return this.terminalService.sendText(text); } - public executeCommand(commandLine: string): Promise { + public async executeCommand(commandLine: string): Promise { return this.terminalService.executeCommand(commandLine); } public show(preserveFocus?: boolean | undefined): Promise { diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index db2b7f80e4b1..0a85e3f38545 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -51,15 +51,20 @@ export interface ITerminalService extends IDisposable { args: string[], cancel?: CancellationToken, swallowExceptions?: boolean, - ): Promise; + ): Promise; /** @deprecated */ sendText(text: string): Promise; - executeCommand(commandLine: string): Promise; + executeCommand(commandLine: string): Promise; show(preserveFocus?: boolean): Promise; } export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory'); +export interface IExecuteCommandResult { + execution: TerminalShellExecution; + exitCode: Promise; +} + export type TerminalCreationOptions = { /** * Object with environment variables that will be added to the Terminal. diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index 3cba6141763b..09c9669e52a6 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -24,7 +24,7 @@ import { sendTelemetryEvent } from '../../telemetry'; export class TerminalCodeExecutionProvider implements ICodeExecutionService { private hasRanOutsideCurrentDrive = false; protected terminalTitle!: string; - private replActive?: Promise; + replActive?: Promise; constructor( @inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory, @@ -73,6 +73,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { this.replActive = new Promise(async (resolve) => { const replCommandArgs = await this.getExecutableInfo(resource); let listener: IDisposable; + // TODO: There's a race condition here as well; the send text from file command may happen before this timeout resolves Promise.race([ new Promise((resolve) => setTimeout(() => resolve(true), 3000)), new Promise((resolve) => { @@ -99,9 +100,17 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { } resolve(true); }); + // python + // python shell, exit code undefined. + // ondidStartShellExecution happens, si fires ondidEndsi - await terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); + // TODO: Store this promise and make sure no further commands are executed without awaiting .exitCode + const replExecution = await terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); // need to make sure this is resolved before starting executing something in repl. + + // TODO: Should we await replyExecution.exitCode here? + await replExecution?.exitCode; }); + this.disposables.push( terminalService.onDidCloseTerminal(() => { this.replActive = undefined; diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 5fd129e8fe89..705606ec722e 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -7,6 +7,7 @@ import { IDisposable, Resource } from '../common/types'; export const ICodeExecutionService = Symbol('ICodeExecutionService'); export interface ICodeExecutionService { + replActive?: Promise; execute(code: string, resource?: Uri): Promise; executeFile(file: Uri, options?: { newTerminalPerFile: boolean }): Promise; initializeRepl(resource?: Uri): Promise; From 2044f831514440a0bf708e4f449f67f9cb78db03 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 11 Nov 2024 11:48:58 -0800 Subject: [PATCH 2/2] figure out if we are inside Python REPL --- src/client/common/terminal/service.ts | 22 ++++++++++++++----- .../common/terminal/syncTerminalService.ts | 7 ++++-- src/client/common/terminal/types.ts | 2 +- .../codeExecution/terminalCodeExecution.ts | 2 +- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index f20ad8496bdf..6ec147c6da7a 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -21,6 +21,7 @@ import { TerminalShellType, } from './types'; import { traceVerbose } from '../../logging'; +import { getConfiguration } from '../vscodeApis/workspaceApis'; @injectable() export class TerminalService implements ITerminalService, Disposable { @@ -69,7 +70,7 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(true); } - return this.executeCommand(text); + return this.executeCommand(text, false); } /** @deprecated */ public async sendText(text: string): Promise { @@ -77,9 +78,12 @@ export class TerminalService implements ITerminalService, Disposable { if (!this.options?.hideFromUser) { this.terminal!.show(true); } - this.terminal!.sendText(text); + this.terminal!.sendText(text, false); } - public async executeCommand(commandLine: string): Promise { + public async executeCommand( + commandLine: string, + isPythonShell: boolean, + ): Promise { const terminal = this.terminal!; if (!this.options?.hideFromUser) { terminal.show(true); @@ -102,12 +106,18 @@ export class TerminalService implements ITerminalService, Disposable { }); await promise; } - // python in a shell , exit code is undefined . startCommand event happen, we call end command event - if (terminal.shellIntegration) { + // If shell integration for python is disabled, use sendText inside REPL regardless of upstream shell integration setting. + const config = getConfiguration('python'); + const pythonrcSetting = config.get('terminal.shellIntegration.enabled'); + if (isPythonShell && !pythonrcSetting) { + terminal.sendText(commandLine); + return undefined; + } else if (terminal.shellIntegration) { + // python in a shell , exit code is undefined . startCommand event happen, we call end command event // TODO: Await the python REPL execute promise here. So we know python repl launched for sure before executing other python code. // So we would not be interrupted. - await this.serviceContainer.get(ICodeExecutionService).replActive; + // await this.serviceContainer.get(ICodeExecutionService).replActive; getting undefined const execution = terminal.shellIntegration.executeCommand(commandLine); traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`); diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts index 265635cf18b6..6a74d8ae674d 100644 --- a/src/client/common/terminal/syncTerminalService.ts +++ b/src/client/common/terminal/syncTerminalService.ts @@ -145,8 +145,11 @@ export class SynchronousTerminalService implements ITerminalService, Disposable public sendText(text: string): Promise { return this.terminalService.sendText(text); } - public async executeCommand(commandLine: string): Promise { - return this.terminalService.executeCommand(commandLine); + public async executeCommand( + commandLine: string, + isPythonShell: boolean, + ): Promise { + return this.terminalService.executeCommand(commandLine, isPythonShell); } public show(preserveFocus?: boolean | undefined): Promise { return this.terminalService.show(preserveFocus); diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index 0a85e3f38545..b782a3b6c79f 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -54,7 +54,7 @@ export interface ITerminalService extends IDisposable { ): Promise; /** @deprecated */ sendText(text: string): Promise; - executeCommand(commandLine: string): Promise; + executeCommand(commandLine: string, isPythonShell: boolean): Promise; show(preserveFocus?: boolean): Promise; } diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index 09c9669e52a6..55a90b701cc6 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -59,7 +59,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource); } } else { - await this.getTerminalService(resource).executeCommand(code); + await this.getTerminalService(resource).executeCommand(code, true); } }