fix(cli): prevent quit flow on Ctrl+C when input has text#27001
fix(cli): prevent quit flow on Ctrl+C when input has text#27001cynthialong0-0 wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the user experience in the interactive CLI by refining the behavior of the Ctrl+C shortcut. Previously, Ctrl+C would trigger the application quit flow regardless of the input state. The changes ensure that if a user has text in their input buffer, the first Ctrl+C press will clear the input instead of exiting the application, aligning with expected terminal behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Size Change: +366 B (0%) Total Size: 34.1 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request updates the Ctrl+C handling in AppContainer.tsx to prevent the application from exiting when the input buffer is not empty. It introduces a mechanism to reset the exit counter and delegate the keypress to the input prompt for clearing text. A unit test was added to validate this logic. The reviewer noted that the implementation relies on Command.QUIT and Command.CLEAR_INPUT sharing a key binding, which might not hold true and could conflict with the Esc-Esc shortcut requirement for clearing input.
| if ( | ||
| keyMatchers[Command.QUIT](key) && | ||
| keyMatchers[Command.CLEAR_INPUT](key) && | ||
| isInputActive && | ||
| streamingState === StreamingState.Idle && | ||
| buffer.text.length > 0 | ||
| ) { |
There was a problem hiding this comment.
The condition keyMatchers[Command.QUIT](key) && keyMatchers[Command.CLEAR_INPUT](key) assumes that both commands are bound to the same key. However, repository rules specify that the Esc-Esc shortcut should be used to clear the input buffer, including when it contains only whitespace. If Command.QUIT is bound to a different key (e.g., Ctrl+C), this condition will never be true. Please ensure the logic correctly handles the Esc-Esc requirement for clearing input.
References
- The Esc-Esc shortcut should clear the input buffer, including when it contains only whitespace.
Summary
Fix Ctrl+C behavior in interactive mode where it trigger quit instead of clearing input.
Details
Update AppContainer.tsso that Ctrl+C direct to edit.clear when active input, idle streaming, non-empty buffer.Related Issues
Fixes #23146
How to Validate
Pre-Merge Checklist