Przeglądaj źródła

Revert "Handle outputless commands"

This reverts commit 710284cc3da1b61b22169b621457d6ae77029ec8 which has
been superseded by PR #1365.

Fixes: #1416
Eric Wheeler 9 miesięcy temu
rodzic
commit
384b469bf1

+ 0 - 6
src/core/Cline.ts

@@ -978,12 +978,6 @@ export class Cline {
 			await this.say("shell_integration_warning")
 		})
 
-		process.once("stream_stalled", async (id: number) => {
-			if (id === terminalInfo.id && !didContinue) {
-				sendCommandOutput("")
-			}
-		})
-
 		await process
 
 		// Wait for a short delay to ensure all messages are sent to the webview.

+ 12 - 31
src/integrations/terminal/TerminalProcess.ts

@@ -41,17 +41,12 @@ export interface TerminalProcessEvents {
 	error: [error: Error]
 	no_shell_integration: []
 	/**
-	 * Emitted when a shell execution completes.
+	 * Emitted when a shell execution completes
 	 * @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<string>]
-	/**
-	 * Emitted when an execution fails to emit a "line" event for a given period of time.
-	 * @param id The terminal ID
-	 */
-	stream_stalled: [id: number]
 }
 
 export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
@@ -60,7 +55,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 	private isListening = true
 	private terminalInfo: TerminalInfo | undefined
-	private lastEmitAt = 0
+	private lastEmitTime_ms = 0
 	private outputBuilder?: OutputBuilder
 	private hotTimer: NodeJS.Timeout | null = null
 
@@ -72,18 +67,14 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 		this._isHot = value
 	}
 
-	constructor(
-		private readonly terminalOutputLimit: number,
-		private readonly stallTimeout: number = 5_000,
-	) {
+	constructor(private readonly terminalOutputLimit: number) {
 		super()
 	}
 
 	async run(terminal: vscode.Terminal, command: string) {
 		if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) {
-			// Get terminal info to access stream.
+			// Get terminal info to access stream
 			const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(terminal)
-
 			if (!terminalInfo) {
 				console.error("[TerminalProcess] Terminal not found in registry")
 				this.emit("no_shell_integration")
@@ -136,9 +127,11 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 			this.outputBuilder = new OutputBuilder({ maxSize: this.terminalOutputLimit })
 
-			let stallTimer: NodeJS.Timeout | null = setTimeout(() => {
-				this.emit("stream_stalled", terminalInfo.id)
-			}, this.stallTimeout)
+			/**
+			 * Some commands won't result in output flushing until the command
+			 * completes. This locks the UI. Should we set a timer to prompt
+			 * the user to continue?
+			 */
 
 			for await (let data of stream) {
 				// Check for command output start marker.
@@ -165,17 +158,11 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 				// right away but this wouldn't happen until it emits a line break, so
 				// as soon as we get any output we emit to let webview know to show spinner.
 				const now = Date.now()
-				const timeSinceLastEmit = now - this.lastEmitAt
+				const timeSinceLastEmit = now - this.lastEmitTime_ms
 
 				if (this.isListening && timeSinceLastEmit > EMIT_INTERVAL) {
-					if (this.flushLine()) {
-						if (stallTimer) {
-							clearTimeout(stallTimer)
-							stallTimer = null
-						}
-
-						this.lastEmitAt = now
-					}
+					this.flushLine()
+					this.lastEmitTime_ms = now
 				}
 
 				// Set isHot depending on the command.
@@ -271,10 +258,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 		if (line) {
 			this.emit("line", line)
-			return true
 		}
-
-		return false
 	}
 
 	private flushAll() {
@@ -286,10 +270,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 		if (buffer) {
 			this.emit("line", buffer)
-			return true
 		}
-
-		return false
 	}
 
 	private processOutput(outputToProcess: string) {

+ 1 - 156
src/integrations/terminal/__tests__/TerminalProcess.test.ts

@@ -20,9 +20,6 @@ jest.mock("vscode", () => ({
 	ThemeIcon: jest.fn(),
 }))
 
-const TERMINAL_OUTPUT_LIMIT = 100 * 1024
-const STALL_TIMEOUT = 100
-
 describe("TerminalProcess", () => {
 	let terminalProcess: TerminalProcess
 	let mockTerminal: jest.Mocked<
@@ -37,7 +34,7 @@ describe("TerminalProcess", () => {
 	let mockStream: AsyncIterableIterator<string>
 
 	beforeEach(() => {
-		terminalProcess = new TerminalProcess(TERMINAL_OUTPUT_LIMIT, STALL_TIMEOUT)
+		terminalProcess = new TerminalProcess(100 * 1024)
 
 		// Create properly typed mock terminal
 		mockTerminal = {
@@ -176,156 +173,4 @@ describe("TerminalProcess", () => {
 			expect(terminalProcess["isListening"]).toBe(false)
 		})
 	})
-
-	describe("stalled stream handling", () => {
-		it("emits stream_stalled event when no output is received within timeout", async () => {
-			// Create a promise that resolves when stream_stalled is emitted
-			const streamStalledPromise = new Promise<number>((resolve) => {
-				terminalProcess.once("stream_stalled", (id: number) => {
-					resolve(id)
-				})
-			})
-
-			// Create a stream that doesn't emit any data
-			mockStream = (async function* () {
-				yield "\x1b]633;C\x07" // Command start sequence
-				// No data is yielded after this, causing the stall
-				await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
-				// This would normally be yielded, but the stall timer will fire first
-				yield "Output after stall"
-				yield "\x1b]633;D\x07" // Command end sequence
-				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
-			})()
-
-			mockExecution = {
-				read: jest.fn().mockReturnValue(mockStream),
-			}
-
-			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
-
-			// Start the terminal process
-			const runPromise = terminalProcess.run(mockTerminal, "test command")
-			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
-
-			// Wait for the stream_stalled event
-			const stalledId = await streamStalledPromise
-
-			// Verify the event was emitted with the correct terminal ID
-			expect(stalledId).toBe(mockTerminalInfo.id)
-
-			// Complete the run
-			await runPromise
-		})
-
-		it("clears stall timer when output is received", async () => {
-			// Spy on the emit method to check if stream_stalled is emitted
-			const emitSpy = jest.spyOn(terminalProcess, "emit")
-
-			// Create a stream that emits data before the stall timeout
-			mockStream = (async function* () {
-				yield "\x1b]633;C\x07" // Command start sequence
-				yield "Initial output\n" // This should clear the stall timer
-
-				// Wait longer than the stall timeout
-				await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
-
-				yield "More output\n"
-				yield "\x1b]633;D\x07" // Command end sequence
-				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
-			})()
-
-			mockExecution = {
-				read: jest.fn().mockReturnValue(mockStream),
-			}
-
-			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
-
-			// Start the terminal process
-			const runPromise = terminalProcess.run(mockTerminal, "test command")
-			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
-
-			// Wait for the run to complete
-			await runPromise
-
-			// Wait a bit longer to ensure the stall timer would have fired if not cleared
-			await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
-
-			// Verify stream_stalled was not emitted
-			expect(emitSpy).not.toHaveBeenCalledWith("stream_stalled", expect.anything())
-		})
-
-		it("returns true from flushLine when a line is emitted", async () => {
-			// Create a stream with output
-			mockStream = (async function* () {
-				yield "\x1b]633;C\x07" // Command start sequence
-				yield "Test output\n" // This should be flushed as a line
-				yield "\x1b]633;D\x07" // Command end sequence
-				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
-			})()
-
-			mockExecution = {
-				read: jest.fn().mockReturnValue(mockStream),
-			}
-
-			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
-
-			// Spy on the flushLine method
-			const flushLineSpy = jest.spyOn(terminalProcess as any, "flushLine")
-
-			// Spy on the emit method to check if line is emitted
-			const emitSpy = jest.spyOn(terminalProcess, "emit")
-
-			// Start the terminal process
-			const runPromise = terminalProcess.run(mockTerminal, "test command")
-			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
-
-			// Wait for the run to complete
-			await runPromise
-
-			// Verify flushLine was called and returned true
-			expect(flushLineSpy).toHaveBeenCalled()
-			expect(flushLineSpy.mock.results.some((result) => result.value === true)).toBe(true)
-
-			// Verify line event was emitted
-			expect(emitSpy).toHaveBeenCalledWith("line", expect.any(String))
-		})
-
-		it("returns false from flushLine when no line is emitted", async () => {
-			// Create a stream with no complete lines
-			mockStream = (async function* () {
-				yield "\x1b]633;C\x07" // Command start sequence
-				yield "Test output" // No newline, so this won't be flushed as a line yet
-				yield "\x1b]633;D\x07" // Command end sequence
-				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
-			})()
-
-			mockExecution = {
-				read: jest.fn().mockReturnValue(mockStream),
-			}
-
-			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
-
-			// Create a custom implementation to test flushLine directly
-			const testFlushLine = async () => {
-				// Create a new instance with the same configuration
-				const testProcess = new TerminalProcess(TERMINAL_OUTPUT_LIMIT, STALL_TIMEOUT)
-
-				// Set up the output builder with content that doesn't have a newline
-				testProcess["outputBuilder"] = {
-					readLine: jest.fn().mockReturnValue(""),
-					append: jest.fn(),
-					reset: jest.fn(),
-					content: "Test output",
-				} as any
-
-				// Call flushLine directly
-				const result = testProcess["flushLine"]()
-				return result
-			}
-
-			// Test flushLine directly
-			const flushLineResult = await testFlushLine()
-			expect(flushLineResult).toBe(false)
-		})
-	})
 })