From 7dadf4cd1ba4ff645efbda97b1cc471b29d44d29 Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Sat, 8 Mar 2025 14:37:12 -0800 Subject: [PATCH] refactor: remove redundant terminal ID from event handling As pointed out by @cte, passing and checking terminal IDs in events is unnecessary since a TerminalProcess instance can never be associated with a different Terminal instance. The event handling is already properly scoped to the specific TerminalProcess instance. - Remove terminal ID parameter from shell_execution_complete event - Remove terminal ID parameter from stream_available event - Update all event handlers to remove ID checks - Update all test cases to match new event signatures Signed-off-by: Eric Wheeler --- src/core/Cline.ts | 6 ++---- src/integrations/terminal/Terminal.ts | 4 ++-- src/integrations/terminal/TerminalProcess.ts | 16 ++++++---------- .../terminal/__tests__/TerminalProcess.test.ts | 8 ++++---- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 6729fa326d..e7ffa5c409 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -982,10 +982,8 @@ export class Cline { completed = true }) - process.once("shell_execution_complete", (id: number, details: ExitCodeDetails) => { - if (id === terminalInfo.id) { - exitDetails = details - } + process.once("shell_execution_complete", (details: ExitCodeDetails) => { + exitDetails = details }) process.once("no_shell_integration", async () => { diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 7233b275b2..23b999e8b8 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -60,7 +60,7 @@ export class Terminal { this.streamClosed = false this.running = true - this.process.emit("stream_available", this.id, stream) + this.process.emit("stream_available", stream) } else { // Stream is being closed this.streamClosed = true @@ -81,7 +81,7 @@ export class Terminal { this.completedProcesses.unshift(this.process) } - this.process.emit("shell_execution_complete", this.id, exitDetails) + this.process.emit("shell_execution_complete", exitDetails) this.process = undefined } } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 2abc3b1333..a5376fd27c 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -108,8 +108,8 @@ export interface TerminalProcessEvents { * @param id The terminal ID * @param exitDetails Contains exit code and signal information if process was terminated by signal */ - shell_execution_complete: [id: number, exitDetails: ExitCodeDetails] - stream_available: [id: number, stream: AsyncIterable] + shell_execution_complete: [exitDetails: ExitCodeDetails] + stream_available: [stream: AsyncIterable] } // how long to wait after a process outputs anything before we consider it "cool" again @@ -247,19 +247,15 @@ export class TerminalProcess extends EventEmitter { if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) { // Create a promise that resolves when the stream becomes available const streamAvailable = new Promise>((resolve) => { - this.once("stream_available", (id: number, stream: AsyncIterable) => { - if (id === this.terminalInfo.id) { - resolve(stream) - } + this.once("stream_available", (stream: AsyncIterable) => { + resolve(stream) }) }) // Create promise that resolves when shell execution completes for this terminal const shellExecutionComplete = new Promise((resolve) => { - this.once("shell_execution_complete", (id: number, exitDetails: ExitCodeDetails) => { - if (id === this.terminalInfo.id) { - resolve(exitDetails) - } + this.once("shell_execution_complete", (exitDetails: ExitCodeDetails) => { + resolve(exitDetails) }) }) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.test.ts b/src/integrations/terminal/__tests__/TerminalProcess.test.ts index a7dad5cc09..b1ac031892 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.test.ts @@ -85,7 +85,7 @@ describe("TerminalProcess", () => { yield "More output\n" yield "Final output" yield "\x1b]633;D\x07" // The last chunk contains the command end sequence with bell character. - terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 }) + terminalProcess.emit("shell_execution_complete", { exitCode: 0 }) })() mockExecution = { @@ -95,7 +95,7 @@ describe("TerminalProcess", () => { mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution) const runPromise = terminalProcess.run("test command") - terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream) + terminalProcess.emit("stream_available", mockStream) await runPromise expect(lines).toEqual(["Initial output", "More output", "Final output"]) @@ -157,7 +157,7 @@ describe("TerminalProcess", () => { yield "still compiling...\n" yield "done" yield "\x1b]633;D\x07" // The last chunk contains the command end sequence with bell character. - terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 }) + terminalProcess.emit("shell_execution_complete", { exitCode: 0 }) })() mockTerminal.shellIntegration.executeCommand.mockReturnValue({ @@ -165,7 +165,7 @@ describe("TerminalProcess", () => { }) const runPromise = terminalProcess.run("npm run build") - terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream) + terminalProcess.emit("stream_available", mockStream) expect(terminalProcess.isHot).toBe(true) await runPromise