-
Notifications
You must be signed in to change notification settings - Fork 36.7k
Closed
Labels
chat-terminalThe run in terminal tool in chatThe run in terminal tool in chatfeature-requestRequest for new features or functionalityRequest for new features or functionalityinsiders-releasedPatch has been released in VS Code InsidersPatch has been released in VS Code Insiderson-testplan
Milestone
Description
The current logic for parsing a full command line into sub-commands is a naive string splitting algorithm using the following list:
vscode/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/subCommands.ts
Lines 17 to 48 in 14978d7
| // Derived from https://github.com/microsoft/vscode/blob/315b0949786b3807f05cb6acd13bf0029690a052/extensions/terminal-suggest/src/tokens.ts#L14-L18 | |
| // Some of these can match the same string, so the order matters. | |
| // | |
| // This isn't perfect, at some point it would be better off moving over to tree sitter for this | |
| // instead of simple string matching. | |
| const shellTypeResetChars = new Map<'sh' | 'zsh' | 'pwsh', string[]>([ | |
| ['sh', sortByStringLengthDesc([ | |
| // Redirection docs (bash) https://www.gnu.org/software/bash/manual/html_node/Redirections.html | |
| ...createNumberRange(1, 9).concat('').map(n => `${n}<<<`), // Here strings | |
| ...createNumberRange(1, 9).concat('').flatMap(n => createNumberRange(1, 9).map(m => `${n}>&${m}`)), // Redirect stream to stream | |
| ...createNumberRange(1, 9).concat('').map(n => `${n}<>`), // Open file descriptor for reading and writing | |
| ...createNumberRange(1, 9).concat('&', '').map(n => `${n}>>`), | |
| ...createNumberRange(1, 9).concat('&', '').map(n => `${n}>`), | |
| '0<', '||', '&&', '|&', '<<', '&', ';', '{', '>', '<', '|' | |
| ])], | |
| ['zsh', sortByStringLengthDesc([ | |
| // Redirection docs https://zsh.sourceforge.io/Doc/Release/Redirection.html | |
| ...createNumberRange(1, 9).concat('').map(n => `${n}<<<`), // Here strings | |
| ...createNumberRange(1, 9).concat('').flatMap(n => createNumberRange(1, 9).map(m => `${n}>&${m}`)), // Redirect stream to stream | |
| ...createNumberRange(1, 9).concat('').map(n => `${n}<>`), // Open file descriptor for reading and writing | |
| ...createNumberRange(1, 9).concat('&', '').map(n => `${n}>>`), | |
| ...createNumberRange(1, 9).concat('&', '').map(n => `${n}>`), | |
| '<(', '||', '>|', '>!', '&&', '|&', '&', ';', '{', '<(', '<', '|' | |
| ])], | |
| ['pwsh', sortByStringLengthDesc([ | |
| // Redirection docs: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_redirection?view=powershell-7.5 | |
| ...createNumberRange(1, 6).concat('*', '').flatMap(n => createNumberRange(1, 6).map(m => `${n}>&${m}`)), // Stream to stream redirection | |
| ...createNumberRange(1, 6).concat('*', '').map(n => `${n}>>`), | |
| ...createNumberRange(1, 6).concat('*', '').map(n => `${n}>`), | |
| '&&', '<', '|', ';', '!', '&' | |
| ])], | |
| ]); |
This should use a proper parser like tree-sitter to parse the command. This would fix cases like echo "a|b|c" being incorrectly treated as sub commands ['echo', 'a', 'b', 'c"'] for example.
dmipeck, linonetwo, kayasax, gvanrossum, PlustOwner and 1 moreCopilot, pamelafox and rseymour
Metadata
Metadata
Labels
chat-terminalThe run in terminal tool in chatThe run in terminal tool in chatfeature-requestRequest for new features or functionalityRequest for new features or functionalityinsiders-releasedPatch has been released in VS Code InsidersPatch has been released in VS Code Insiderson-testplan