From 7e0a4dd426fef2c1a56d54f79341857939694ad3 Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Fri, 7 Mar 2025 19:02:12 -0800 Subject: [PATCH] Revert "Try to prevent additional cases in which terminal commands lock the task UI" This reverts commit eee7bbe104eff25a434854c91b8bb4cf92d02dcb which has been superseded by PR #1365. Fixes: #1435 --- src/integrations/terminal/TerminalManager.ts | 62 +++++++------------- src/integrations/terminal/TerminalProcess.ts | 45 +++----------- 2 files changed, 28 insertions(+), 79 deletions(-) diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts index 04201fd4ff..6dd0a57c46 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -82,10 +82,7 @@ declare module "vscode" { // https://github.com/microsoft/vscode/blob/f0417069c62e20f3667506f4b7e53ca0004b4e3e/src/vscode-dts/vscode.d.ts#L10794 interface Window { onDidStartTerminalShellExecution?: ( - listener: (e: { - terminal: vscode.Terminal - execution: { read(): AsyncIterable; commandLine: { value: string } } - }) => any, + listener: (e: any) => any, thisArgs?: any, disposables?: vscode.Disposable[], ) => vscode.Disposable @@ -206,77 +203,57 @@ export class TerminalManager { constructor() { let startDisposable: vscode.Disposable | undefined let endDisposable: vscode.Disposable | undefined - try { // onDidStartTerminalShellExecution startDisposable = (vscode.window as vscode.Window).onDidStartTerminalShellExecution?.(async (e) => { // Get a handle to the stream as early as possible: const stream = e?.execution.read() const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(e.terminal) + if (stream && terminalInfo) { + const process = this.processes.get(terminalInfo.id) + if (process) { + terminalInfo.stream = stream + terminalInfo.running = true + terminalInfo.streamClosed = false + process.emit("stream_available", terminalInfo.id, stream) + } + } else { + console.error("[TerminalManager] Stream failed, not registered for terminal") + } - console.info("[TerminalManager] shell execution started", { + console.info("[TerminalManager] Shell execution started:", { hasExecution: !!e?.execution, - hasStream: !!stream, command: e?.execution?.commandLine?.value, terminalId: terminalInfo?.id, }) - - if (terminalInfo) { - const process = this.processes.get(terminalInfo.id) - - if (process) { - if (stream) { - terminalInfo.stream = stream - terminalInfo.running = true - terminalInfo.streamClosed = false - console.log(`[TerminalManager] stream_available -> ${terminalInfo.id}`) - process.emit("stream_available", terminalInfo.id, stream) - } else { - process.emit("stream_unavailable", terminalInfo.id) - console.error(`[TerminalManager] stream_unavailable -> ${terminalInfo.id}`) - } - } - } else { - console.error("[TerminalManager] terminalInfo not available") - } }) // onDidEndTerminalShellExecution endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => { const exitDetails = this.interpretExitCode(e?.exitCode) - console.info("[TerminalManager] Shell execution ended:", { ...exitDetails }) - let emitted = false + console.info("[TerminalManager] Shell execution ended:", { + ...exitDetails, + }) - // Signal completion to any waiting processes. + // Signal completion to any waiting processes for (const id of this.terminalIds) { const info = TerminalRegistry.getTerminal(id) - if (info && info.terminal === e.terminal) { info.running = false const process = this.processes.get(id) - if (process) { - console.log(`[TerminalManager] emitting shell_execution_complete -> ${id}`) - emitted = true process.emit("shell_execution_complete", id, exitDetails) } - break } } - - if (!emitted) { - console.log(`[TerminalManager#onDidStartTerminalShellExecution] no terminal found`) - } }) } catch (error) { - console.error("[TerminalManager] failed to configure shell execution handlers", error) + console.error("[TerminalManager] Error setting up shell execution handlers:", error) } - if (startDisposable) { this.disposables.push(startDisposable) } - if (endDisposable) { this.disposables.push(endDisposable) } @@ -389,6 +366,9 @@ export class TerminalManager { } disposeAll() { + // for (const info of this.terminals) { + // //info.terminal.dispose() // dont want to dispose terminals when task is aborted + // } this.terminalIds.clear() this.processes.clear() this.disposables.forEach((disposable) => disposable.dispose()) diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index db9eeefd99..b31185dcc5 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -47,7 +47,6 @@ export interface TerminalProcessEvents { */ shell_execution_complete: [id: number, exitDetails: ExitCodeDetails] stream_available: [id: number, stream: AsyncIterable] - stream_unavailable: [id: number] /** * Emitted when an execution fails to emit a "line" event for a given period of time. * @param id The terminal ID @@ -86,24 +85,15 @@ export class TerminalProcess extends EventEmitter { const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(terminal) if (!terminalInfo) { - console.error("[TerminalProcess#run] terminal not found in registry") + console.error("[TerminalProcess] Terminal not found in registry") this.emit("no_shell_integration") this.emit("completed") this.emit("continue") return } - this.once("stream_unavailable", (id: number) => { - if (id === terminalInfo.id) { - console.error(`[TerminalProcess#run] stream_unavailable`) - this.emit("completed") - this.emit("continue") - } - }) - - // When `executeCommand()` is called, `onDidStartTerminalShellExecution` - // will fire in `TerminalManager` which creates a new stream via - // `execution.read()` and emits `stream_available`. + // When executeCommand() is called, onDidStartTerminalShellExecution will fire in TerminalManager + // which creates a new stream via execution.read() and emits 'stream_available' const streamAvailable = new Promise>((resolve) => { this.once("stream_available", (id: number, stream: AsyncIterable) => { if (id === terminalInfo.id) { @@ -121,35 +111,15 @@ export class TerminalProcess extends EventEmitter { }) }) - // `readLine()` needs to know if streamClosed, so store this for later. - // NOTE: This doesn't seem to be used anywhere. + // readLine needs to know if streamClosed, so store this for later this.terminalInfo = terminalInfo - // Execute command. + // Execute command terminal.shellIntegration.executeCommand(command) this.isHot = true - // Wait for stream to be available. - // const stream = await streamAvailable - - // Wait for stream to be available. - let stream: AsyncIterable - - try { - stream = await Promise.race([ - streamAvailable, - new Promise((_, reject) => { - setTimeout( - () => reject(new Error("Timeout waiting for terminal stream to become available")), - 10_000, - ) - }), - ]) - } catch (error) { - console.error(`[TerminalProcess#run] timed out waiting for stream`) - this.emit("stream_stalled", terminalInfo.id) - stream = await streamAvailable - } + // Wait for stream to be available + const stream = await streamAvailable let preOutput = "" let commandOutputStarted = false @@ -286,7 +256,6 @@ export class TerminalProcess extends EventEmitter { } public continue() { - console.log(`[TerminalProcess#continue] flushing all`) this.flushAll() this.isListening = false this.removeAllListeners("line")