-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat: Add terminal instance selection for @terminal context mention #10634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add TerminalInfo interface and terminalSearchResults message type - Update context-mentions.ts to show terminal sub-selection menu - Add searchTerminals handler in webviewMessageHandler.ts - Update ChatTextArea.tsx to fetch terminals when Terminal type selected - Update parseMentions to handle @Terminal:name syntax - Update getLatestTerminalOutput to support specific terminal selection - Update openMention to focus specific terminal when clicked Addresses #10622
Reviewed the terminal instance selection feature. Found 2 issues to address:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| if (selectedType === ContextMenuOptionType.Terminal) { | ||
| const terminals = queryItems.filter((item) => item.type === ContextMenuOptionType.Terminal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filtering will include duplicate entries. The queryItems array already contains a default { type: Terminal, value: "terminal" } entry (added in ChatTextArea.tsx line 297), and then you prepend another "All Terminals" option with the same value. When the terminal sub-menu is displayed, users will see two items that both represent "All Terminals" - one with an explicit label and one that falls back to default rendering.
Consider filtering out the default terminal item before adding the explicit "All Terminals" option:
| if (selectedType === ContextMenuOptionType.Terminal) { | |
| const terminals = queryItems.filter((item) => item.type === ContextMenuOptionType.Terminal) | |
| if (selectedType === ContextMenuOptionType.Terminal) { | |
| const terminals = queryItems.filter( | |
| (item) => item.type === ContextMenuOptionType.Terminal && item.value !== "terminal" | |
| ) |
Fix it with Roo Code or mention @roomote and request a fix.
| // Show the terminal to make it active | ||
| targetTerminal.show() | ||
| // Small delay to ensure terminal is focused | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 100ms delay is an arbitrary heuristic that may not reliably ensure the terminal is focused before selectAll executes. Terminal.show() makes the terminal visible but doesn't guarantee it becomes the active terminal synchronously. On slower machines or busy VS Code instances, this timing may be insufficient, potentially causing content to be copied from the wrong terminal.
Consider using a polling approach with vscode.window.activeTerminal to verify the terminal is actually active, or document this as a known limitation. A more robust pattern might be:
targetTerminal.show()
const maxRetries = 10
for (let i = 0; i < maxRetries; i++) {
if (vscode.window.activeTerminal === targetTerminal) break
await new Promise(resolve => setTimeout(resolve, 50))
}Fix it with Roo Code or mention @roomote and request a fix.
Summary
This PR attempts to address Issue #10622. Feedback and guidance are welcome.
Problem
When users have multiple terminal tabs open, the
@terminalcontext mention grabs output from all terminals indiscriminately, leading to context pollution and wasted tokens from unrelated terminal sessions.Solution
This PR adds the ability to select a specific terminal instance when using the
@terminalcontext mention:Terminal sub-selection menu: When selecting
@terminalin the context menu, users now see a sub-menu listing all available terminal instances (similar to git commit selection), plus an "All Terminals" option for the previous behaviorNew mention syntax: Added support for
@terminal:namesyntax to reference a specific terminal by name (e.g.,@terminal:bash,@terminal:zsh)Specific terminal output: When using
@terminal:name, only the output from that specific terminal is included in the contextChanges
TerminalInfointerface andterminalSearchResults/searchTerminalsmessage typesgetContextMenuOptions()searchTerminalshandler to return terminal instancesparseMentionsandgetLatestTerminalOutputto handle@terminal:namesyntaxTesting
Closes #10622
Important
Adds terminal instance selection for
@terminalcontext mention with sub-menu and@terminal:namesyntax, updating relevant code and UI components.@terminalcontext mention with a sub-menu and@terminal:namesyntax.@terminal:name.vscode-extension-host.ts: AddsTerminalInfointerface andterminalSearchResults/searchTerminalsmessage types.context-mentions.ts: UpdatesgetContextMenuOptions()to include terminal sub-selection.webviewMessageHandler.ts: AddssearchTerminalshandler to return terminal instances.ChatTextArea.tsx: Fetches terminal instances when Terminal type is selected.This description was created by
for a6253d6. You can customize this summary. It will automatically update as commits are pushed.