-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Format python commands specially #288358
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
Format python commands specially #288358
Conversation
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.
Pull request overview
This PR adds special formatting for Python commands executed via python -c "..." syntax in terminal tool confirmations. Instead of showing the full shell command with escaped quotes, the UI now displays just the Python code with proper syntax highlighting, making it easier for users to understand what will be executed.
Changes:
- Added
extractPythonCommandfunction to parse and extract Python code frompython -ccommands with proper quote unescaping for different shells (bash vs PowerShell) - Added
presentationOverridesto the terminal tool data model to support alternative display formatting without changing the actual command that runs - Modified UI components (confirmation and progress parts) to use presentationOverrides when available, including making the editor read-only when displaying extracted Python code
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts |
Implements the extractPythonCommand function with shell-aware quote unescaping |
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts |
Integrates Python command extraction into the tool preparation flow and updates confirmation titles |
src/vs/workbench/contrib/chat/common/chatService/chatService.ts |
Adds presentationOverrides interface to the terminal tool invocation data model |
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts |
Uses presentationOverrides for displaying command with appropriate language in the progress part |
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts |
Uses presentationOverrides for confirmation display and makes editor read-only when showing extracted Python |
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts |
Comprehensive test suite for extractPythonCommand covering various quote styles, shells, and edge cases |
Comments suppressed due to low confidence (3)
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts:80
- Similar to the double quote regex, this pattern uses a greedy
.+which will fail with trailing content and doesn't handle absolute paths to python executables. Consider using a non-greedy match and allowing for paths.
const singleQuoteMatch = commandLine.match(/^python(?:3)?\s+-c\s+'(?<python>.+)'$/s);
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts:193
- When presentationOverrides is set and the editor is read-only, this edit handler should not prepend the cdPrefix since the displayed content is the extracted Python code, not the actual command with cd prefix. The cdPrefix logic only makes sense when displaying the actual shell command. This could lead to incorrect command reconstruction.
terminalData.commandLine.userEdited = cdPrefix + currentValue;
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts:82
- The
.trim()operation removes leading/trailing whitespace from the Python code. However, in Python, leading whitespace is significant for indentation. While trailing whitespace is generally fine to trim, leading whitespace should be preserved as it may be intentional. Consider only using.trimEnd()or handling this more carefully.
return singleQuoteMatch.groups.python.trim();
Fixes #287772