Fix ask question dialog to allow it to take the whole window height if needed.#23693
Fix ask question dialog to allow it to take the whole window height if needed.#23693
Conversation
|
Hi @jacob314, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
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 refines the user interface by enhancing the 'Ask Question' dialog's ability to display extensive content and by optimizing the rendering of tool outputs. The changes ensure that the UI adapts better to different terminal sizes and handles the display of various tool states, including hidden or ongoing operations, with greater precision and visual clarity. 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 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 counter productive. 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: +544 B (0%) Total Size: 26.2 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request refactors the rendering logic for tool groups and dialogs within the CLI UI. Key changes include updating the AskUserDialog to allow questions to exceed previous line limits in tall terminals and refining the visibility logic for tool calls using a new shouldHideToolCall utility. The changes also introduce comprehensive regression tests for ToolGroupMessage to address issues with phantom tool groups, spurious bridge groups, and correct border rendering for agents. Snapshot tests for ExitPlanModeDialog and MainContent have been updated to reflect these rendering improvements, particularly for hidden lines in plan mode. A review comment points out a potential issue in ToolGroupMessage.tsx where the bottom border for an empty tool group might render spuriously if it's not explicitly closing an existing box, suggesting a refinement to the rendering condition.
| (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) && | ||
| borderBottomOverride !== false && ( |
There was a problem hiding this comment.
The logic for rendering the bottom border of an empty tool group (a "closing slice") appears to be incomplete. It currently renders a bottom border if borderBottomOverride is not false, without considering if it's a continuation of a previous box (i.e., if borderTop is true).
This can lead to spurious horizontal lines in the UI when an empty tool group is rendered without a preceding open box. The new regression test Plan Mode: suppresses spurious bridge group (empty, no top border) seems designed to catch this, but the current implementation would still render a border.
The condition should be strengthened to only render the border for an empty group if it's meant to close an existing box, which is indicated by borderTopOverride being true.
| (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) && | |
| borderBottomOverride !== false && ( | |
| (visibleToolCalls.length > 0 || (borderBottomOverride !== undefined && borderTopOverride)) && | |
| borderBottomOverride !== false && ( |
Summary
Details
Related Issues
How to Validate
Pre-Merge Checklist