From bf2ce7e1eea2c4b861dfa593d257b2f78068a3bb Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Wed, 5 Mar 2025 18:35:00 -0800 Subject: [PATCH] refactor: move terminal functionality to Terminal class Move terminal lifecycle management to improve organization: 1. Move runCommand to Terminal class 2. Move getOrCreateTerminal to TerminalRegistry Signed-off-by: Eric Wheeler --- src/core/Cline.ts | 2 +- src/integrations/terminal/Terminal.ts | 36 ++++++++- src/integrations/terminal/TerminalManager.ts | 76 +++---------------- src/integrations/terminal/TerminalRegistry.ts | 36 +++++++++ 4 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 8ac333c1f6..2a2ea88033 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -923,7 +923,7 @@ export class Cline { async executeCommandTool(command: string): Promise<[boolean, ToolResponse]> { const terminalInfo = await this.terminalManager.getOrCreateTerminal(cwd) terminalInfo.terminal.show() // weird visual bug when creating new terminals (even manually) where there's an empty space at the top. - const process = this.terminalManager.runCommand(terminalInfo, command) + const process = terminalInfo.runCommand(command) let userFeedback: { text?: string; images?: string[] } | undefined let didContinue = false diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 8140d27fbb..ac6ad99233 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode" -import { ExitCodeDetails, TerminalProcess } from "./TerminalProcess" +import pWaitFor from "p-wait-for" +import { ExitCodeDetails, mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess" export class Terminal { public terminal: vscode.Terminal @@ -71,6 +72,39 @@ export class Terminal { } } + public runCommand(command: string): TerminalProcessResultPromise { + this.busy = true + this.lastCommand = command + + // Create process immediately + const process = new TerminalProcess(this) + + // Set process on terminal + this.process = process + + // Create a promise for command completion + const promise = new Promise((resolve, reject) => { + // Set up event handlers + process.once("continue", () => resolve()) + process.once("error", (error) => { + console.error(`Error in terminal ${this.id}:`, error) + reject(error) + }) + + // Wait for shell integration before executing the command + pWaitFor(() => this.terminal.shellIntegration !== undefined, { timeout: 4000 }) + .then(() => { + process.run(command) + }) + .catch(() => { + console.log("[Terminal] Shell integration not available. Command execution aborted.") + process.emit("no_shell_integration") + }) + }) + + return mergePromise(process, promise) + } + /** * Gets the terminal contents based on the number of commands to include * @param commands Number of previous commands to include (-1 for all) diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts index 4b2a78db6d..2fce2a4fcf 100644 --- a/src/integrations/terminal/TerminalManager.ts +++ b/src/integrations/terminal/TerminalManager.ts @@ -34,10 +34,13 @@ Windows: pwsh Example: -const terminalManager = new TerminalManager(context); +const terminalManager = new TerminalManager(); + +// Get a terminal for the project directory +const terminal = await terminalManager.getTerminal('/path/to/project'); // Run a command -const process = terminalManager.runCommand('npm install', '/path/to/project'); +const process = terminal.runCommand('npm install'); process.on('line', (line) => { console.log(line); @@ -50,7 +53,7 @@ await process; process.continue(); // Later, if you need to get the unretrieved output: -const unretrievedOutput = terminalManager.getUnretrievedOutput(terminalId); +const unretrievedOutput = TerminalRegistry.getUnretrievedOutput(terminal.id); console.log('Unretrieved output:', unretrievedOutput); Resources: @@ -98,71 +101,10 @@ declare module "vscode" { export class TerminalManager { private terminalIds: Set = new Set() - runCommand(terminalInfo: Terminal, command: string): TerminalProcessResultPromise { - terminalInfo.busy = true - terminalInfo.lastCommand = command - - // Create process immediately - const process = new TerminalProcess(terminalInfo) - - // Set process on terminal - terminalInfo.process = process - - // Create a promise for command completion - const promise = new Promise((resolve, reject) => { - // Set up event handlers - process.once("continue", () => resolve()) - process.once("error", (error) => { - console.error(`Error in terminal ${terminalInfo.id}:`, error) - reject(error) - }) - - // Wait for shell integration before executing the command - pWaitFor(() => terminalInfo.terminal.shellIntegration !== undefined, { timeout: 4000 }) - .then(() => { - process.run(command) - }) - .catch(() => { - console.log("[TerminalManager] Shell integration not available. Command execution aborted.") - process.emit("no_shell_integration") - }) - }) - - return mergePromise(process, promise) - } - async getOrCreateTerminal(cwd: string): Promise { - const terminals = TerminalRegistry.getAllTerminals() - - // Find available terminal from our pool first (created for this task) - const matchingTerminal = terminals.find((t) => { - if (t.busy) { - return false - } - const terminalCwd = t.terminal.shellIntegration?.cwd // one of cline's commands could have changed the cwd of the terminal - if (!terminalCwd) { - return false - } - return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath) - }) - if (matchingTerminal) { - this.terminalIds.add(matchingTerminal.id) - return matchingTerminal - } - - // If no matching terminal exists, try to find any non-busy terminal - const availableTerminal = terminals.find((t) => !t.busy) - if (availableTerminal) { - // Navigate back to the desired directory - await this.runCommand(availableTerminal, `cd "${cwd}"`) - this.terminalIds.add(availableTerminal.id) - return availableTerminal - } - - // If all terminals are busy, create a new one - const newTerminalInfo = TerminalRegistry.createTerminal(cwd) - this.terminalIds.add(newTerminalInfo.id) - return newTerminalInfo + const terminal = await TerminalRegistry.getOrCreateTerminal(cwd) + this.terminalIds.add(terminal.id) + return terminal } disposeAll() { diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 50d89de9c3..3f0d2170f5 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -1,4 +1,5 @@ import * as vscode from "vscode" +import { arePathsEqual } from "../../utils/path" import { Terminal } from "./Terminal" import { TerminalProcess } from "./TerminalProcess" @@ -174,4 +175,39 @@ export class TerminalRegistry { this.disposables.forEach((disposable) => disposable.dispose()) this.disposables = [] } + + /** + * Gets an existing terminal or creates a new one for the given working directory + * @param cwd The working directory path + * @returns A Terminal instance + */ + static async getOrCreateTerminal(cwd: string): Promise { + const terminals = this.getAllTerminals() + + // Find available terminal from our pool first (created for this task) + const matchingTerminal = terminals.find((t) => { + if (t.busy) { + return false + } + const terminalCwd = t.terminal.shellIntegration?.cwd // one of cline's commands could have changed the cwd of the terminal + if (!terminalCwd) { + return false + } + return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath) + }) + if (matchingTerminal) { + return matchingTerminal + } + + // If no matching terminal exists, try to find any non-busy terminal + const availableTerminal = terminals.find((t) => !t.busy) + if (availableTerminal) { + // Navigate back to the desired directory + await availableTerminal.runCommand(`cd "${cwd}"`) + return availableTerminal + } + + // If all terminals are busy, create a new one + return this.createTerminal(cwd) + } }