Skip to content

Commit f03465a

Browse files
committed
feat(terminal): replace pWaitFor with event-based shell integration wait; add cmd.exe fast-path
1 parent 5bcd678 commit f03465a

2 files changed

Lines changed: 108 additions & 24 deletions

File tree

src/__mocks__/vscode.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export const window = {
8181
sendText: () => {},
8282
}),
8383
onDidCloseTerminal: () => mockDisposable,
84+
onDidChangeTerminalShellIntegration: () => mockDisposable,
8485
createTextEditorDecorationType: () => ({ dispose: () => {} }),
8586
}
8687

src/integrations/terminal/Terminal.ts

Lines changed: 107 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { existsSync } from "fs"
22
import * as path from "path"
33

44
import * as vscode from "vscode"
5-
import pWaitFor from "p-wait-for"
65

76
import type { RooTerminalCallbacks, RooTerminalProcessResultPromise } from "./types"
87
import { BaseTerminal } from "./BaseTerminal"
@@ -49,7 +48,9 @@ export class Terminal extends BaseTerminal {
4948
this.terminal = vscode.window.createTerminal(options)
5049
}
5150

52-
if (Terminal.getTerminalZdotdir()) {
51+
// Only register ZDOTDIR cleanup when we actually set it (i.e. no profile
52+
// override is active — see getEnv() for the same guard).
53+
if (Terminal.getTerminalZdotdir() && !Terminal.getTerminalProfile()) {
5354
ShellIntegrationManager.terminalTmpDirs.set(id, env.ZDOTDIR)
5455
}
5556
}
@@ -87,7 +88,7 @@ export class Terminal extends BaseTerminal {
8788
process.once("completed", (output) => callbacks.onCompleted(output, process))
8889
process.once("shell_execution_started", (pid) => callbacks.onShellExecutionStarted(pid, process))
8990
process.once("shell_execution_complete", (details) => callbacks.onShellExecutionComplete(details, process))
90-
process.once("no_shell_integration", (msg) => callbacks.onNoShellIntegration?.(msg, process))
91+
process.once("no_shell_integration", (details) => callbacks.onNoShellIntegration?.(details, process))
9192

9293
const promise = new Promise<void>((resolve, reject) => {
9394
// Set up event handlers
@@ -97,28 +98,60 @@ export class Terminal extends BaseTerminal {
9798
reject(error)
9899
})
99100

100-
// Wait for shell integration before executing the command
101-
pWaitFor(() => this.terminal.shellIntegration !== undefined, {
102-
timeout: Terminal.getShellIntegrationTimeout(),
103-
})
104-
.then(() => {
105-
// Clean up temporary directory if shell integration is available, zsh did its job:
106-
ShellIntegrationManager.zshCleanupTmpDir(this.id)
101+
// Wait for shell integration before executing the command. Use the
102+
// event-based API rather than polling so we react immediately when
103+
// VS Code's injector fires instead of burning CPU on a tight loop.
104+
const waitForShellIntegration = (): Promise<void> => {
105+
if (this.terminal.shellIntegration !== undefined) {
106+
return Promise.resolve()
107+
}
107108

108-
// Run the command in the terminal
109-
process.run(command)
109+
return new Promise<void>((res, rej) => {
110+
const timeoutId = setTimeout(() => {
111+
disposable.dispose()
112+
rej(new Error("timeout"))
113+
}, Terminal.getShellIntegrationTimeout())
114+
115+
const disposable = vscode.window.onDidChangeTerminalShellIntegration(({ terminal }) => {
116+
if (terminal === this.terminal) {
117+
clearTimeout(timeoutId)
118+
disposable.dispose()
119+
res()
120+
}
121+
})
110122
})
111-
.catch(() => {
112-
console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`)
113-
114-
// Clean up temporary directory if shell integration is not available
115-
ShellIntegrationManager.zshCleanupTmpDir(this.id)
123+
}
116124

117-
process.emit(
118-
"no_shell_integration",
119-
`Shell integration initialization sequence '\\x1b]633;A' was not received within ${Terminal.getShellIntegrationTimeout() / 1000}s. Shell integration has been disabled for this terminal instance. Increase the timeout in the settings if necessary.`,
120-
)
125+
if (Terminal.isActiveShellCmdExe()) {
126+
// cmd.exe cannot emit OSC 633;A — skip the timeout entirely and go
127+
// straight to the execa fallback (VS Code issue #164646).
128+
ShellIntegrationManager.zshCleanupTmpDir(this.id)
129+
process.emit("no_shell_integration", {
130+
message:
131+
"cmd.exe does not support shell integration (VS Code issue #164646). Command will run via fallback.",
132+
commandSubmitted: false,
121133
})
134+
} else {
135+
waitForShellIntegration()
136+
.then(() => {
137+
// Clean up temporary directory if shell integration is available, zsh did its job:
138+
ShellIntegrationManager.zshCleanupTmpDir(this.id)
139+
140+
// Run the command in the terminal
141+
process.run(command)
142+
})
143+
.catch(() => {
144+
console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`)
145+
146+
// Clean up temporary directory if shell integration is not available
147+
ShellIntegrationManager.zshCleanupTmpDir(this.id)
148+
149+
process.emit("no_shell_integration", {
150+
message: `Shell integration initialization sequence '\\x1b]633;A' was not received within ${Terminal.getShellIntegrationTimeout() / 1000}s. Shell integration has been disabled for this terminal instance. Increase the timeout in the settings if necessary.`,
151+
commandSubmitted: false,
152+
})
153+
})
154+
}
122155
})
123156

124157
return mergePromise(process, promise)
@@ -214,8 +247,11 @@ export class Terminal extends BaseTerminal {
214247
env.PROMPT_EOL_MARK = ""
215248
}
216249

217-
// Handle ZDOTDIR for zsh if enabled
218-
if (Terminal.getTerminalZdotdir()) {
250+
// Handle ZDOTDIR for zsh if enabled. Skip when a profile override is
251+
// active: VS Code's own shell integration injector also sets ZDOTDIR for
252+
// zsh, and the two would fight each other (VS Code's ambient env wins per
253+
// issue #96295). Let VS Code handle injection for the selected profile.
254+
if (Terminal.getTerminalZdotdir() && !Terminal.getTerminalProfile()) {
219255
env.ZDOTDIR = ShellIntegrationManager.zshInitTmpDir(env)
220256
}
221257

@@ -310,6 +346,52 @@ export class Terminal extends BaseTerminal {
310346
}
311347
}
312348

349+
/**
350+
* Returns true when the resolved shell path is cmd.exe. cmd.exe cannot emit
351+
* the OSC 633;C sequence (VS Code issue #164646, closed as not planned), so
352+
* shell integration will never work for it — exclude it from the picker.
353+
*/
354+
public static isCmdExe(shellPath: string): boolean {
355+
return /[/\\]cmd\.exe$/i.test(shellPath)
356+
}
357+
358+
/**
359+
* Returns true when the active shell (profile override or VS Code default) is
360+
* cmd.exe. Used to skip the shell integration timeout entirely for cmd.exe.
361+
*/
362+
public static isActiveShellCmdExe(platform: NodeJS.Platform = process.platform): boolean {
363+
if (platform !== "win32") {
364+
return false
365+
}
366+
367+
// Check explicit profile override first.
368+
const profileShell = Terminal.getProfileShell(platform)
369+
370+
if (profileShell?.shellPath) {
371+
return Terminal.isCmdExe(profileShell.shellPath)
372+
}
373+
374+
// Fall back to VS Code's configured default profile for Windows.
375+
const platformKey = Terminal.getPlatformProfileKey(platform)
376+
const defaultProfileName = vscode.workspace
377+
.getConfiguration("terminal.integrated")
378+
.get<string>(`defaultProfile.${platformKey}`)
379+
380+
if (!defaultProfileName) {
381+
return false
382+
}
383+
384+
const profiles = Terminal.getConfiguredProfiles(platform)
385+
const profile = profiles[defaultProfileName] as { path?: unknown } | null | undefined
386+
387+
if (!profile) {
388+
return false
389+
}
390+
391+
const resolved = Terminal.resolveProfilePath(profile.path, platform)
392+
return resolved ? Terminal.isCmdExe(resolved) : false
393+
}
394+
313395
public static getAvailableProfileNames(platform: NodeJS.Platform = process.platform): string[] {
314396
const names = new Set<string>()
315397

@@ -319,8 +401,9 @@ export class Terminal extends BaseTerminal {
319401
}
320402

321403
const { path: profilePath } = entry as { path?: unknown }
404+
const resolved = Terminal.resolveProfilePath(profilePath, platform)
322405

323-
if (Terminal.resolveProfilePath(profilePath, platform)) {
406+
if (resolved && !Terminal.isCmdExe(resolved)) {
324407
names.add(name)
325408
}
326409
}

0 commit comments

Comments
 (0)