feat(cli): improve CTRL+O experience for both standard and alternate screen buffer (ASB) modes#19010
Conversation
Summary of ChangesHello @jwhelangoog, 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 enhances the Gemini CLI's standard terminal mode by introducing a feature that allows users to dynamically expand and collapse shell command output using Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a great enhancement to the user experience in standard terminal mode by allowing shell output to be expanded with Ctrl+O. The implementation is solid, with well-reasoned changes across AppContainer, MainContent, and ShellToolMessage. The logic to handle height constraints and trigger re-renders in standard mode via refreshStatic is correctly implemented and conditioned to not affect alternate buffer mode. The updates to memoization dependencies in MainContent ensure the UI reacts correctly to the new state. The new tests in ShellToolMessage.test.tsx effectively validate the expansion behavior. Overall, this is a well-executed feature addition. I have not found any issues of high or critical severity.
|
I have reviewed the PR and identified a potentially significant design issue related to the standard terminal mode experience.
Is this acceptable behavior for standard terminal mode? Alternatively, we could consider:
I've also audited the React logic and tests. The Let me know how you'd like to proceed with the |
|
This review was generated by Here are a few potential issues and observations, particularly keeping the 1. Missing Cleanup Function in
|
|
To address the feedback:
|
fb6bba2 to
d93cf67
Compare
d93cf67 to
21cf0bd
Compare
jacob314
left a comment
There was a problem hiding this comment.
Approved. I'll follow up with code review comments as a PR.
TLDR: main things I will fix in the follow up
- flicker of text in alternate buffer mode on changing overflow
- fix alternatebufferquittingmessage regression
- Consider if we can simply the changes for overflow. definite codesmell that app container is now basically a defacto overflow provider and changes to render.tsx are not ideal either.
Summary
This PR improves the CTRL+O (Expansion Mode) feature, making it a consistent, flicker-free tool for both standard and alternate screen buffer (ASB) modes. It centralizes layout logic to ensure visual consistency and implements synchronous overflow detection to eliminate UI "pop-in."
Details
Standard Terminal Mode: Turn-Based Expansion
Alternate Screen Buffer (ASB) Mode: Managed Viewport Expansion
constrainHeightstate, allowing for full expansion within the scrollable viewport.calculateShellMaxLines) now returnundefinedwhen unconstrained, enabling components to provide a fluid, full-screen navigation experience.Technical Polish: Centralized Layout & Flicker Reduction
toolLayoutUtils.tsto unify height and overflow calculations. This ensures thatToolGroupMessage(for overflow detection) andToolResultDisplay(for actual truncation) stay perfectly in sync.useMemo. By detecting overflow during render instead of waiting for aResizeObserver, the expansion hint (ShowMoreLines) appears atomically with the content, eliminating UI flicker.Related Issues
Fixes #17438
How to Validate
Standard Terminal Mode
seq 100).Alternate Screen Buffer (ASB) Mode
seq 100.Pre-Merge Checklist