Skip to content

Commit 7e0a4dd

Browse files
author
Eric Wheeler
committed
Revert "Try to prevent additional cases in which terminal commands lock the task UI"
This reverts commit eee7bbe which has been superseded by PR #1365. Fixes: #1435
1 parent edb53bf commit 7e0a4dd

File tree

2 files changed

+28
-79
lines changed

2 files changed

+28
-79
lines changed

src/integrations/terminal/TerminalManager.ts

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,7 @@ declare module "vscode" {
8282
// https://github.com/microsoft/vscode/blob/f0417069c62e20f3667506f4b7e53ca0004b4e3e/src/vscode-dts/vscode.d.ts#L10794
8383
interface Window {
8484
onDidStartTerminalShellExecution?: (
85-
listener: (e: {
86-
terminal: vscode.Terminal
87-
execution: { read(): AsyncIterable<string>; commandLine: { value: string } }
88-
}) => any,
85+
listener: (e: any) => any,
8986
thisArgs?: any,
9087
disposables?: vscode.Disposable[],
9188
) => vscode.Disposable
@@ -206,77 +203,57 @@ export class TerminalManager {
206203
constructor() {
207204
let startDisposable: vscode.Disposable | undefined
208205
let endDisposable: vscode.Disposable | undefined
209-
210206
try {
211207
// onDidStartTerminalShellExecution
212208
startDisposable = (vscode.window as vscode.Window).onDidStartTerminalShellExecution?.(async (e) => {
213209
// Get a handle to the stream as early as possible:
214210
const stream = e?.execution.read()
215211
const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(e.terminal)
216-
217-
console.info("[TerminalManager] shell execution started", {
218-
hasExecution: !!e?.execution,
219-
hasStream: !!stream,
220-
command: e?.execution?.commandLine?.value,
221-
terminalId: terminalInfo?.id,
222-
})
223-
224-
if (terminalInfo) {
212+
if (stream && terminalInfo) {
225213
const process = this.processes.get(terminalInfo.id)
226-
227214
if (process) {
228-
if (stream) {
229-
terminalInfo.stream = stream
230-
terminalInfo.running = true
231-
terminalInfo.streamClosed = false
232-
console.log(`[TerminalManager] stream_available -> ${terminalInfo.id}`)
233-
process.emit("stream_available", terminalInfo.id, stream)
234-
} else {
235-
process.emit("stream_unavailable", terminalInfo.id)
236-
console.error(`[TerminalManager] stream_unavailable -> ${terminalInfo.id}`)
237-
}
215+
terminalInfo.stream = stream
216+
terminalInfo.running = true
217+
terminalInfo.streamClosed = false
218+
process.emit("stream_available", terminalInfo.id, stream)
238219
}
239220
} else {
240-
console.error("[TerminalManager] terminalInfo not available")
221+
console.error("[TerminalManager] Stream failed, not registered for terminal")
241222
}
223+
224+
console.info("[TerminalManager] Shell execution started:", {
225+
hasExecution: !!e?.execution,
226+
command: e?.execution?.commandLine?.value,
227+
terminalId: terminalInfo?.id,
228+
})
242229
})
243230

244231
// onDidEndTerminalShellExecution
245232
endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => {
246233
const exitDetails = this.interpretExitCode(e?.exitCode)
247-
console.info("[TerminalManager] Shell execution ended:", { ...exitDetails })
248-
let emitted = false
234+
console.info("[TerminalManager] Shell execution ended:", {
235+
...exitDetails,
236+
})
249237

250-
// Signal completion to any waiting processes.
238+
// Signal completion to any waiting processes
251239
for (const id of this.terminalIds) {
252240
const info = TerminalRegistry.getTerminal(id)
253-
254241
if (info && info.terminal === e.terminal) {
255242
info.running = false
256243
const process = this.processes.get(id)
257-
258244
if (process) {
259-
console.log(`[TerminalManager] emitting shell_execution_complete -> ${id}`)
260-
emitted = true
261245
process.emit("shell_execution_complete", id, exitDetails)
262246
}
263-
264247
break
265248
}
266249
}
267-
268-
if (!emitted) {
269-
console.log(`[TerminalManager#onDidStartTerminalShellExecution] no terminal found`)
270-
}
271250
})
272251
} catch (error) {
273-
console.error("[TerminalManager] failed to configure shell execution handlers", error)
252+
console.error("[TerminalManager] Error setting up shell execution handlers:", error)
274253
}
275-
276254
if (startDisposable) {
277255
this.disposables.push(startDisposable)
278256
}
279-
280257
if (endDisposable) {
281258
this.disposables.push(endDisposable)
282259
}
@@ -389,6 +366,9 @@ export class TerminalManager {
389366
}
390367

391368
disposeAll() {
369+
// for (const info of this.terminals) {
370+
// //info.terminal.dispose() // dont want to dispose terminals when task is aborted
371+
// }
392372
this.terminalIds.clear()
393373
this.processes.clear()
394374
this.disposables.forEach((disposable) => disposable.dispose())

src/integrations/terminal/TerminalProcess.ts

Lines changed: 7 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ export interface TerminalProcessEvents {
4747
*/
4848
shell_execution_complete: [id: number, exitDetails: ExitCodeDetails]
4949
stream_available: [id: number, stream: AsyncIterable<string>]
50-
stream_unavailable: [id: number]
5150
/**
5251
* Emitted when an execution fails to emit a "line" event for a given period of time.
5352
* @param id The terminal ID
@@ -86,24 +85,15 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
8685
const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(terminal)
8786

8887
if (!terminalInfo) {
89-
console.error("[TerminalProcess#run] terminal not found in registry")
88+
console.error("[TerminalProcess] Terminal not found in registry")
9089
this.emit("no_shell_integration")
9190
this.emit("completed")
9291
this.emit("continue")
9392
return
9493
}
9594

96-
this.once("stream_unavailable", (id: number) => {
97-
if (id === terminalInfo.id) {
98-
console.error(`[TerminalProcess#run] stream_unavailable`)
99-
this.emit("completed")
100-
this.emit("continue")
101-
}
102-
})
103-
104-
// When `executeCommand()` is called, `onDidStartTerminalShellExecution`
105-
// will fire in `TerminalManager` which creates a new stream via
106-
// `execution.read()` and emits `stream_available`.
95+
// When executeCommand() is called, onDidStartTerminalShellExecution will fire in TerminalManager
96+
// which creates a new stream via execution.read() and emits 'stream_available'
10797
const streamAvailable = new Promise<AsyncIterable<string>>((resolve) => {
10898
this.once("stream_available", (id: number, stream: AsyncIterable<string>) => {
10999
if (id === terminalInfo.id) {
@@ -121,35 +111,15 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
121111
})
122112
})
123113

124-
// `readLine()` needs to know if streamClosed, so store this for later.
125-
// NOTE: This doesn't seem to be used anywhere.
114+
// readLine needs to know if streamClosed, so store this for later
126115
this.terminalInfo = terminalInfo
127116

128-
// Execute command.
117+
// Execute command
129118
terminal.shellIntegration.executeCommand(command)
130119
this.isHot = true
131120

132-
// Wait for stream to be available.
133-
// const stream = await streamAvailable
134-
135-
// Wait for stream to be available.
136-
let stream: AsyncIterable<string>
137-
138-
try {
139-
stream = await Promise.race([
140-
streamAvailable,
141-
new Promise<never>((_, reject) => {
142-
setTimeout(
143-
() => reject(new Error("Timeout waiting for terminal stream to become available")),
144-
10_000,
145-
)
146-
}),
147-
])
148-
} catch (error) {
149-
console.error(`[TerminalProcess#run] timed out waiting for stream`)
150-
this.emit("stream_stalled", terminalInfo.id)
151-
stream = await streamAvailable
152-
}
121+
// Wait for stream to be available
122+
const stream = await streamAvailable
153123

154124
let preOutput = ""
155125
let commandOutputStarted = false
@@ -286,7 +256,6 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
286256
}
287257

288258
public continue() {
289-
console.log(`[TerminalProcess#continue] flushing all`)
290259
this.flushAll()
291260
this.isListening = false
292261
this.removeAllListeners("line")

0 commit comments

Comments
 (0)