From 3a950bbc15e2600da4cbcf8f4fa7311e903fdafb Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Mon, 3 Mar 2025 21:00:23 -0800 Subject: [PATCH] refactor: Rename TerminalInfo to Terminal and relocate to Terminal.ts Transformed the TerminalInfo interface into a proper Terminal class and moved it to its own file. This improves code organization and encapsulation by centralizing terminal-related functionality. The change establishes a clearer object model for terminal management, setting the foundation for a more maintainable terminal architecture. All references throughout the codebase have been updated to use the new Terminal class while preserving existing functionality. Tests have been updated and verified to ensure compatibility with the new structure. Signed-off-by: Eric Wheeler --- src/integrations/terminal/Terminal.ts | 20 +++++++++++ src/integrations/terminal/TerminalManager.ts | 9 ++--- src/integrations/terminal/TerminalProcess.ts | 5 +-- src/integrations/terminal/TerminalRegistry.ts | 36 ++++++------------- .../__tests__/TerminalProcess.test.ts | 14 +++----- .../__tests__/TerminalProcessExec.test.ts | 12 ++----- 6 files changed, 45 insertions(+), 51 deletions(-) create mode 100644 src/integrations/terminal/Terminal.ts diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts new file mode 100644 index 0000000000..b8a51bc08a --- /dev/null +++ b/src/integrations/terminal/Terminal.ts @@ -0,0 +1,20 @@ +import * as vscode from "vscode" + +export class Terminal { + public terminal: vscode.Terminal + public busy: boolean + public lastCommand: string + public id: number + public stream?: AsyncIterable + public running: boolean + public streamClosed: boolean + + constructor(id: number, terminal: vscode.Terminal) { + this.id = id + this.terminal = terminal + this.busy = false + this.lastCommand = "" + this.running = false + this.streamClosed = false + } +} diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts index a55f7867d4..5bf87784ae 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -2,7 +2,8 @@ import pWaitFor from "p-wait-for" import * as vscode from "vscode" import { arePathsEqual } from "../../utils/path" import { mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess" -import { TerminalInfo, TerminalRegistry } from "./TerminalRegistry" +import { Terminal } from "./Terminal" +import { TerminalRegistry } from "./TerminalRegistry" /* TerminalManager: @@ -259,7 +260,7 @@ export class TerminalManager { } } - runCommand(terminalInfo: TerminalInfo, command: string): TerminalProcessResultPromise { + runCommand(terminalInfo: Terminal, command: string): TerminalProcessResultPromise { terminalInfo.busy = true terminalInfo.lastCommand = command const process = new TerminalProcess() @@ -306,7 +307,7 @@ export class TerminalManager { return mergePromise(process, promise) } - async getOrCreateTerminal(cwd: string): Promise { + async getOrCreateTerminal(cwd: string): Promise { const terminals = TerminalRegistry.getAllTerminals() // Find available terminal from our pool first (created for this task) @@ -343,7 +344,7 @@ export class TerminalManager { getTerminals(busy: boolean): { id: number; lastCommand: string }[] { return Array.from(this.terminalIds) .map((id) => TerminalRegistry.getTerminal(id)) - .filter((t): t is TerminalInfo => t !== undefined && t.busy === busy) + .filter((t): t is Terminal => t !== undefined && t.busy === busy) .map((t) => ({ id: t.id, lastCommand: t.lastCommand })) } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 99ef215e78..fa578d890f 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -4,7 +4,8 @@ import * as vscode from "vscode" import { inspect } from "util" import { ExitCodeDetails } from "./TerminalManager" -import { TerminalInfo, TerminalRegistry } from "./TerminalRegistry" +import { Terminal } from "./Terminal" +import { TerminalRegistry } from "./TerminalRegistry" export interface TerminalProcessEvents { line: [line: string] @@ -28,7 +29,7 @@ const PROCESS_HOT_TIMEOUT_COMPILING = 15_000 export class TerminalProcess extends EventEmitter { waitForShellIntegration: boolean = true private isListening: boolean = true - private terminalInfo: TerminalInfo | undefined + private terminalInfo: Terminal | undefined private lastEmitTime_ms: number = 0 private fullOutput: string = "" private lastRetrievedIndex: number = 0 diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 69a21d94fd..1689190631 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -1,22 +1,13 @@ import * as vscode from "vscode" - -export interface TerminalInfo { - terminal: vscode.Terminal - busy: boolean - lastCommand: string - id: number - stream?: AsyncIterable - running: boolean - streamClosed: boolean -} +import { Terminal } from "./Terminal" // Although vscode.window.terminals provides a list of all open terminals, there's no way to know whether they're busy or not (exitStatus does not provide useful information for most commands). In order to prevent creating too many terminals, we need to keep track of terminals through the life of the extension, as well as session specific terminals for the life of a task (to get latest unretrieved output). // Since we have promises keeping track of terminal processes, we get the added benefit of keep track of busy terminals even after a task is closed. export class TerminalRegistry { - private static terminals: TerminalInfo[] = [] + private static terminals: Terminal[] = [] private static nextTerminalId = 1 - static createTerminal(cwd?: string | vscode.Uri | undefined): TerminalInfo { + static createTerminal(cwd?: string | vscode.Uri | undefined): Terminal { const terminal = vscode.window.createTerminal({ cwd, name: "Roo Code", @@ -35,20 +26,13 @@ export class TerminalRegistry { }, }) - const newInfo: TerminalInfo = { - terminal, - busy: false, - lastCommand: "", - id: this.nextTerminalId++, - running: false, - streamClosed: false, - } + const newTerminal = new Terminal(this.nextTerminalId++, terminal) - this.terminals.push(newInfo) - return newInfo + this.terminals.push(newTerminal) + return newTerminal } - static getTerminal(id: number): TerminalInfo | undefined { + static getTerminal(id: number): Terminal | undefined { const terminalInfo = this.terminals.find((t) => t.id === id) if (terminalInfo && this.isTerminalClosed(terminalInfo.terminal)) { @@ -59,7 +43,7 @@ export class TerminalRegistry { return terminalInfo } - static updateTerminal(id: number, updates: Partial) { + static updateTerminal(id: number, updates: Partial) { const terminal = this.getTerminal(id) if (terminal) { @@ -67,7 +51,7 @@ export class TerminalRegistry { } } - static getTerminalInfoByTerminal(terminal: vscode.Terminal): TerminalInfo | undefined { + static getTerminalInfoByTerminal(terminal: vscode.Terminal): Terminal | undefined { const terminalInfo = this.terminals.find((t) => t.terminal === terminal) if (terminalInfo && this.isTerminalClosed(terminalInfo.terminal)) { @@ -82,7 +66,7 @@ export class TerminalRegistry { this.terminals = this.terminals.filter((t) => t.id !== id) } - static getAllTerminals(): TerminalInfo[] { + static getAllTerminals(): Terminal[] { this.terminals = this.terminals.filter((t) => !this.isTerminalClosed(t.terminal)) return this.terminals } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.test.ts b/src/integrations/terminal/__tests__/TerminalProcess.test.ts index 44cae92580..55569a7529 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.test.ts @@ -3,7 +3,8 @@ import * as vscode from "vscode" import { TerminalProcess, mergePromise } from "../TerminalProcess" -import { TerminalInfo, TerminalRegistry } from "../TerminalRegistry" +import { Terminal } from "../Terminal" +import { TerminalRegistry } from "../TerminalRegistry" // Mock vscode.window.createTerminal const mockCreateTerminal = jest.fn() @@ -29,7 +30,7 @@ describe("TerminalProcess", () => { } } > - let mockTerminalInfo: TerminalInfo + let mockTerminalInfo: Terminal let mockExecution: any let mockStream: AsyncIterableIterator @@ -58,14 +59,7 @@ describe("TerminalProcess", () => { } > - mockTerminalInfo = { - terminal: mockTerminal, - busy: false, - lastCommand: "", - id: 1, - running: false, - streamClosed: false, - } + mockTerminalInfo = new Terminal(1, mockTerminal) TerminalRegistry["terminals"].push(mockTerminalInfo) diff --git a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts index 8378f289b7..1c02abbf91 100644 --- a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts @@ -3,7 +3,8 @@ import * as vscode from "vscode" import { execSync } from "child_process" import { TerminalProcess } from "../TerminalProcess" -import { TerminalInfo, TerminalRegistry } from "../TerminalRegistry" +import { Terminal } from "../Terminal" +import { TerminalRegistry } from "../TerminalRegistry" import { TerminalManager } from "../TerminalManager" // Mock the vscode module @@ -100,14 +101,7 @@ async function testTerminalCommand( } // Create terminal info - const mockTerminalInfo: TerminalInfo = { - terminal: mockTerminal, - busy: false, - lastCommand: "", - id: 1, - running: false, - streamClosed: false, - } + const mockTerminalInfo = new Terminal(1, mockTerminal) // Add the terminal to the registry TerminalRegistry["terminals"] = [mockTerminalInfo]