-
Notifications
You must be signed in to change notification settings - Fork 37.3k
Clear prompt input value when onDidEndShellExecution #286791
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
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 addresses a bug where Ctrl+C is incorrectly shown when running Python files after activation commands in the terminal. The issue occurs because the prompt input model retains stale command values between shell executions, particularly on Windows systems with conpty.
Key changes:
- Adds a
clearValue()method to theIPromptInputModelinterface andPromptInputModelclass to clear prompt input state - Calls
clearValue()in theonDidEndTerminalShellExecutionevent handler to prevent stale input values from persisting between commands
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/vs/platform/terminal/common/capabilities/commandDetection/promptInputModel.ts | Adds clearValue() method to clear the prompt input value after command execution |
| src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts | Calls clearValue() when shell execution ends to prevent stale input from showing in subsequent commands |
| * and the value is no longer relevant as editable prompt input. | ||
| */ | ||
| clearValue(): void { | ||
| this._value = ''; |
Copilot
AI
Jan 9, 2026
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 clearValue method only clears the _value field but leaves other state fields like _cursorIndex, _ghostTextIndex, _state, _commandStartMarker, and _commandStartX unchanged. This creates an inconsistent state where the value is empty but other tracking fields still reference the old command. Consider also resetting these fields or at minimum _cursorIndex and _ghostTextIndex to maintain state consistency. Looking at _handleCommandStart (line 219-220), it clears both _value and _cursorIndex together.
| this._value = ''; | |
| this._value = ''; | |
| this._cursorIndex = 0; | |
| this._ghostTextIndex = -1; |
| * and the value is no longer relevant as editable prompt input. | ||
| */ | ||
| clearValue(): void { | ||
| this._value = ''; |
Copilot
AI
Jan 9, 2026
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 clearValue method doesn't trigger any events (onDidChangeInput) after clearing the value. This means subscribers won't be notified of the state change. Compare this to _handleCommandExecuted (line 267) which fires onDidChangeInput after modifying state, and _handleCommandStart (line 222) which also fires events. If extensions or other components are listening to these events to track the prompt state, they won't be notified when the value is cleared, potentially causing them to have stale state as well.
| this._value = ''; | |
| this._value = ''; | |
| this._onDidChangeInput.fire(this._createStateObject()); |
| currentCommand = undefined; | ||
| const instanceId = e.instance.instanceId; | ||
| instanceDataListeners.get(instanceId)?.dispose(); | ||
| // Clear the prompt input value before sending the event to the extension host | ||
| // so that subsequent executeCommand calls don't see stale input | ||
| e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.clearValue(); |
Copilot
AI
Jan 9, 2026
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.
Calling clearValue() on the promptInputModel in the onCommandFinished event handler could cause timing issues. According to the PR description, the problem occurs when an activation command finishes and a Python file command is about to run. The clearValue is called during onCommandFinished which happens after onCommandExecuted (see line 246-268 where _handleCommandExecuted transitions state to Execute and fires events). However, the next command hasn't started yet (which would call _handleCommandStart at line 211-234 and properly initialize state). This creates a window where the model is in an inconsistent state between commands. The timing issue mentioned in the PR description about needing a 500ms timeout suggests this workaround may not fully solve the race condition.
| currentCommand = undefined; | |
| const instanceId = e.instance.instanceId; | |
| instanceDataListeners.get(instanceId)?.dispose(); | |
| // Clear the prompt input value before sending the event to the extension host | |
| // so that subsequent executeCommand calls don't see stale input | |
| e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.clearValue(); | |
| const activeCommand = currentCommand; | |
| const instanceId = e.instance.instanceId; | |
| instanceDataListeners.get(instanceId)?.dispose(); | |
| // Clear the prompt input value before sending the event to the extension host | |
| // so that subsequent executeCommand calls don't see stale input. Only clear if | |
| // no newer command has become active since this one started. | |
| if (!activeCommand || activeCommand.commandLine === e.data.command) { | |
| e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.clearValue(); | |
| } | |
| currentCommand = undefined; |
| clearValue(): void { | ||
| this._value = ''; | ||
| } |
Copilot
AI
Jan 9, 2026
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 new clearValue method lacks test coverage. There is a comprehensive test file at src/vs/platform/terminal/test/common/capabilities/commandDetection/promptInputModel.test.ts with tests for other prompt input model behaviors. Consider adding tests that verify clearValue properly clears the state and handles edge cases like calling clearValue multiple times or calling it in different states (Input, Execute, Unknown).
|
Much better when we clear prompt input at |
Resolves: microsoft/vscode-python-environments#640
Assumption: activate command always run first before python file command.
This is now fact thanks to microsoft/vscode-python-environments#1088
When
python-envs.terminal.autoActivationTypeuscommandwith"python.useEnvironmentsExtension": true,:---> Ctrl+c is displayed when pressing the run button to run Python file.
Specific location of this ctrl+c is after activate command, but before python file execution command.
What is happening:
onDidTerminalShellExecutionEndwhere we wait for activation command (this should be the first thing that runs in the terminal)executeCommand()again (to run Python file.) This is when user presses the run button to run python file.Maybe conpty related bugs, hence why we've been getting more reports from windows users.
Decoration location is off too.
How to resolve (below are options we can take):
onDidEndShellExecution(this PR).Image of the problem: