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 introduces a scrollable interface for the input prompt, significantly improving the user experience when dealing with long, multi-line inputs. By leveraging a virtualized list approach, the UI can now efficiently render and navigate large text buffers. The changes include robust scroll management to keep the cursor in view, better integration with the existing text buffer state, and refinements to the copy mode functionality to ensure consistent behavior across different terminal states. 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: +98 B (0%) Total Size: 34 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request refactors the InputPrompt component to use a ScrollableList for handling large multiline inputs, ensuring the cursor remains visible during interaction. It also introduces a selection mode for the alternate buffer and updates the CopyModeWarning accordingly. Feedback highlights a regression in VirtualizedList where the removal of the copyModeEnabled prop breaks terminal-native selection in other components, and identifies an unsafe ref mutation during the render phase that should be moved to an effect.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/cli/src/ui/components/shared/VirtualizedList.tsx (39-43)
The copyModeEnabled prop has been removed from the VirtualizedListProps interface, and the corresponding logic for handling terminal-native selection has been deleted. This is a critical regression because VirtualizedList is a shared component used by the main chat history view. Copy mode relies on disabling internal scrolling and using a negative marginTop to allow the terminal to perform native text selection. Please restore this prop and the associated rendering logic to maintain consistency with existing UI behavior across components, as non-standard UX changes should be addressed holistically.
References
- Maintain consistency with existing UI behavior across components. Defer non-standard UX pattern improvements to be addressed holistically rather than in a single component.
packages/cli/src/ui/components/shared/VirtualizedList.tsx (274-298)
Mutating prevOffsetsLength.current directly during the render phase is unsafe and violates React's purity requirements. This mutation should be moved to a useLayoutEffect. Additionally, for complex layout calculations involving offsets and scroll anchors, please add detailed comments explaining how these values are derived to prevent incorrect refactoring.
const [prevTargetScrollIndex, setPrevTargetScrollIndex] = useState(
props.targetScrollIndex,
);
const prevOffsetsLength = useRef(offsets.length);
useLayoutEffect(() => {
prevOffsetsLength.current = offsets.length;
});
if (
(props.targetScrollIndex !== undefined &&
props.targetScrollIndex !== prevTargetScrollIndex &&
offsets.length > 1) ||
(props.targetScrollIndex !== undefined &&
prevOffsetsLength.current <= 1 &&
offsets.length > 1)
) {
if (props.targetScrollIndex !== prevTargetScrollIndex) {
setPrevTargetScrollIndex(props.targetScrollIndex);
}
setIsStickingToBottom(false);
setScrollAnchor({ index: props.targetScrollIndex, offset: 0 });
}
References
- For complex layout calculations that depend on component rendering logic (like conditional borders or padding), add detailed comments explaining how the height is derived to prevent incorrect refactoring.
Summary
Fixes #24666
Bugs in copy mode notification text not showing up.
Also fix VirtualizedList behavior in copy mode to stop flickering.