From 9f5a67ecf8e8ddc96d360231b1addddad3dd85b0 Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Wed, 5 Mar 2025 19:28:12 -0800 Subject: [PATCH] refactor: remove TerminalManager after migration - Delete TerminalManager.ts as functionality has been migrated - Remove TerminalManager import and usage from tests - Remove outdated TerminalManager references from comments - Fix TypeScript types in TerminalRegistry event handlers Signed-off-by: Eric Wheeler --- src/integrations/terminal/TerminalManager.ts | 113 ------------------ src/integrations/terminal/TerminalProcess.ts | 4 +- src/integrations/terminal/TerminalRegistry.ts | 60 +++++----- .../__tests__/TerminalProcessExec.test.ts | 9 +- 4 files changed, 35 insertions(+), 151 deletions(-) delete mode 100644 src/integrations/terminal/TerminalManager.ts diff --git a/src/integrations/terminal/TerminalManager.ts b/src/integrations/terminal/TerminalManager.ts deleted file mode 100644 index 2fce2a4fcf..0000000000 --- a/src/integrations/terminal/TerminalManager.ts +++ /dev/null @@ -1,113 +0,0 @@ -import pWaitFor from "p-wait-for" -import * as vscode from "vscode" -import { arePathsEqual } from "../../utils/path" -import { ExitCodeDetails, mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess" -import { Terminal } from "./Terminal" -import { TerminalRegistry } from "./TerminalRegistry" - -/* -TerminalManager: -- Creates/reuses terminals -- Runs commands via runCommand(), returning a TerminalProcess -- Handles shell integration events - -TerminalProcess extends EventEmitter and implements Promise: -- Emits 'line' events with output while promise is pending -- process.continue() resolves promise and stops event emission -- Allows real-time output handling or background execution - -getUnretrievedOutput() fetches latest output for ongoing commands - -Enables flexible command execution: -- Await for completion -- Listen to real-time events -- Continue execution in background -- Retrieve missed output later - -Notes: -- it turns out some shellIntegration APIs are available on cursor, although not on older versions of vscode -- "By default, the shell integration script should automatically activate on supported shells launched from VS Code." -Supported shells: -Linux/macOS: bash, fish, pwsh, zsh -Windows: pwsh - - -Example: - -const terminalManager = new TerminalManager(); - -// Get a terminal for the project directory -const terminal = await terminalManager.getTerminal('/path/to/project'); - -// Run a command -const process = terminal.runCommand('npm install'); - -process.on('line', (line) => { - console.log(line); -}); - -// To wait for the process to complete naturally: -await process; - -// Or to continue execution even if the command is still running: -process.continue(); - -// Later, if you need to get the unretrieved output: -const unretrievedOutput = TerminalRegistry.getUnretrievedOutput(terminal.id); -console.log('Unretrieved output:', unretrievedOutput); - -Resources: -- https://github.com/microsoft/vscode/issues/226655 -- https://code.visualstudio.com/updates/v1_93#_terminal-shell-integration-api -- https://code.visualstudio.com/docs/terminal/shell-integration -- https://code.visualstudio.com/api/references/vscode-api#Terminal -- https://github.com/microsoft/vscode-extension-samples/blob/main/terminal-sample/src/extension.ts -- https://github.com/microsoft/vscode-extension-samples/blob/main/shell-integration-sample/src/extension.ts -*/ - -/* -The new shellIntegration API gives us access to terminal command execution output handling. -However, we don't update our VSCode type definitions or engine requirements to maintain compatibility -with older VSCode versions. Users on older versions will automatically fall back to using sendText -for terminal command execution. -Interestingly, some environments like Cursor enable these APIs even without the latest VSCode engine. -This approach allows us to leverage advanced features when available while ensuring broad compatibility. -*/ -declare module "vscode" { - // https://github.com/microsoft/vscode/blob/f0417069c62e20f3667506f4b7e53ca0004b4e3e/src/vscode-dts/vscode.d.ts#L7442 - // interface Terminal { - // shellIntegration?: { - // cwd?: vscode.Uri - // executeCommand?: (command: string) => { - // read: () => AsyncIterable - // } - // } - // } - // https://github.com/microsoft/vscode/blob/f0417069c62e20f3667506f4b7e53ca0004b4e3e/src/vscode-dts/vscode.d.ts#L10794 - interface Window { - onDidStartTerminalShellExecution?: ( - listener: (e: any) => any, - thisArgs?: any, - disposables?: vscode.Disposable[], - ) => vscode.Disposable - onDidEndTerminalShellExecution?: ( - listener: (e: { terminal: vscode.Terminal; exitCode?: number; shellType?: string }) => any, - thisArgs?: any, - disposables?: vscode.Disposable[], - ) => vscode.Disposable - } -} - -export class TerminalManager { - private terminalIds: Set = new Set() - - async getOrCreateTerminal(cwd: string): Promise { - const terminal = await TerminalRegistry.getOrCreateTerminal(cwd) - this.terminalIds.add(terminal.id) - return terminal - } - - disposeAll() { - this.terminalIds.clear() - } -} diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index ae7a36dfd6..f0a0e5ea6d 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -55,7 +55,6 @@ export class TerminalProcess extends EventEmitter { if (this.terminalInfo) { console.log(`no_shell_integration received for terminal ${this.terminalInfo.id}`) TerminalRegistry.removeTerminal(this.terminalInfo.id) - // Note: TerminalManager.terminalIds cleanup would need to be handled } }) } @@ -159,8 +158,7 @@ export class TerminalProcess extends EventEmitter { const terminal = this.terminalInfo.terminal if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) { - // When executeCommand() is called, onDidStartTerminalShellExecution will fire in TerminalManager - // which creates a new stream via execution.read() and emits 'stream_available' + // 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) { diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 5e9123c45a..04611ccc3f 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -19,37 +19,43 @@ export class TerminalRegistry { try { // onDidStartTerminalShellExecution - const 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 = this.getTerminalByVSCETerminal(e.terminal) - if (terminalInfo) { - terminalInfo.setActiveStream(stream) - } else { - console.error("[TerminalRegistry] Stream failed, not registered for terminal") - } + const startDisposable = vscode.window.onDidStartTerminalShellExecution?.( + async (e: vscode.TerminalShellExecutionStartEvent) => { + // Get a handle to the stream as early as possible: + const stream = e?.execution.read() + const terminalInfo = this.getTerminalByVSCETerminal(e.terminal) + if (terminalInfo) { + terminalInfo.setActiveStream(stream) + } else { + console.error("[TerminalRegistry] Stream failed, not registered for terminal") + } - console.info("[TerminalRegistry] Shell execution started:", { - hasExecution: !!e?.execution, - command: e?.execution?.commandLine?.value, - terminalId: terminalInfo?.id, - }) - }) + console.info("[TerminalRegistry] Shell execution started:", { + hasExecution: !!e?.execution, + command: e?.execution?.commandLine?.value, + terminalId: terminalInfo?.id, + }) + }, + ) // onDidEndTerminalShellExecution - const endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => { - const terminalInfo = this.getTerminalByVSCETerminal(e.terminal) - const process = terminalInfo?.process - const exitDetails = process ? TerminalProcess.interpretExitCode(e?.exitCode) : { exitCode: e?.exitCode } - console.info("[TerminalRegistry] Shell execution ended:", { - ...exitDetails, - }) + const endDisposable = vscode.window.onDidEndTerminalShellExecution?.( + async (e: vscode.TerminalShellExecutionEndEvent) => { + const terminalInfo = this.getTerminalByVSCETerminal(e.terminal) + const process = terminalInfo?.process + const exitDetails = process + ? TerminalProcess.interpretExitCode(e?.exitCode) + : { exitCode: e?.exitCode } + console.info("[TerminalRegistry] Shell execution ended:", { + ...exitDetails, + }) - // Signal completion to any waiting processes - if (terminalInfo) { - terminalInfo.shellExecutionComplete(exitDetails) - } - }) + // Signal completion to any waiting processes + if (terminalInfo) { + terminalInfo.shellExecutionComplete(exitDetails) + } + }, + ) if (startDisposable) { this.disposables.push(startDisposable) diff --git a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts index dddbcca921..287d5361de 100644 --- a/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcessExec.test.ts @@ -5,8 +5,6 @@ import { execSync } from "child_process" import { TerminalProcess, ExitCodeDetails } from "../TerminalProcess" import { Terminal } from "../Terminal" import { TerminalRegistry } from "../TerminalRegistry" -import { TerminalManager } from "../TerminalManager" - // Mock the vscode module jest.mock("vscode", () => { // Store event handlers so we can trigger them in tests @@ -137,9 +135,6 @@ async function testTerminalCommand( // Add the terminal to the registry TerminalRegistry["terminals"] = [mockTerminalInfo] - // Create a terminal manager (this will set up the event handlers) - const terminalManager = new TerminalManager() - // Create a new terminal process for testing startTime = process.hrtime.bigint() // Start timing from terminal process creation const terminalProcess = new TerminalProcess(mockTerminalInfo) @@ -177,9 +172,8 @@ async function testTerminalCommand( }) }) - // Set the process on the terminal and add terminal ID to manager + // Set the process on the terminal mockTerminalInfo.process = terminalProcess - terminalManager["terminalIds"].add(mockTerminalInfo.id) // Run the command (now handled by constructor) // We've already created the process, so we'll trigger the events manually @@ -235,7 +229,6 @@ async function testTerminalCommand( } finally { // Clean up terminalProcess.removeAllListeners() - terminalManager.disposeAll() TerminalRegistry["terminals"] = [] } }