-
Notifications
You must be signed in to change notification settings - Fork 35.6k
Description
@rebornix saw the behavior, when calling terminal.shellIntegration
's executeCommand
duplicate commands can show.
Video of repro:
https://github.com/user-attachments/assets/a5c5abe8-6328-4344-9697-bbb3801cde4b
However, this was avoidable when there was a explicit timeout of 5 seconds before the call to executeCommand
.
The duplicate command showing up behavior is the same behavior we see when user first opens the terminal, and tries to type things before the prompt even shows up.
- In this case, the expected behavior is that we actually want these commands to show up before the prompt does, so the user has feedback of what they have just typed in real-time.
This is the 2s timeout we currently have:
vscode/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
Lines 924 to 934 in 6a66031
await Promise.race([ | |
new Promise<void>(r => { | |
store.add(this.capabilities.onDidAddCapabilityType(e => { | |
if (e === TerminalCapability.CommandDetection) { | |
commandDetection = this.capabilities.get(TerminalCapability.CommandDetection); | |
r(); | |
} | |
})); | |
}), | |
timeout(2000), | |
]); |
And here is what we do in chat terminal:
vscode/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Lines 75 to 82 in 6a66031
const configuredTimeout = this._configurationService.getValue(TerminalChatAgentToolsSettingId.ShellIntegrationTimeout) as number | undefined; | |
let waitTime: number; | |
if (configuredTimeout === undefined || typeof configuredTimeout !== 'number' || configuredTimeout < 0) { | |
waitTime = siInjectionEnabled ? 5000 : (instance.isRemote ? 3000 : 2000); | |
} else { | |
// There's an absolute minimum is 500ms | |
waitTime = Math.max(configuredTimeout, 500); | |
} |
We could move this setting under terminal.integrated.shellIntegration
and wait for however long time defined
via setting, when executeCommand
is called right after startup.
From @rebornix:
"""
Maybe we just completely differentiate between user typing to run command vs. executeCommand
called from extension, and wait however long to prevent the dup command to show when executeCommand
is called from extension.
"""
My concern for this one here is that things may feel slower to user in a sense the prompt and command will only show when (shell integration&its executeCommand and the second one from in the dup command) is ready.
Without upper bound defined, I'm worried something will go wrong in between shell integration being available and executeCommand being called, and user having to wait forever.
/cc @Tyriar