Repository navigation
feat(ui): add dynamic toggle for alternate buffer mode - #29262
bhalsodnikunjhiteshbhai wants to merge 1 commit into
Conversation
- Prevents memory access out of bounds crash in yoga-wasm by avoiding synchronous unmounts of history items - Eliminates duplicated footer artifacts when exiting alternate buffer by ensuring Ink has time to issue erasure codes before swapping terminal buffers - Fully restores normal buffer scrollback without clipping or duplicating history - Resolves startup layout rendering bug when alternate buffer is set in config by manually handling terminal transitions instead of letting Ink's alternateBuffer renderer take over
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/XL
|
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 implements a dynamic toggle for the alternate buffer mode, allowing users to switch between standard and alternate screen buffers using Alt+A. The changes focus on improving the stability of terminal transitions, ensuring that scrollback history is preserved, and fixing layout rendering bugs during startup. By centralizing the alternate buffer state and improving cleanup logic, the PR provides a more robust and responsive terminal UI experience. 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
|
![gemini-code-assist[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to toggle the alternate screen buffer dynamically using the Alt+A shortcut, along with a smooth transition mechanism (isTransitioningAltBuffer) to prevent scrollback erasure when exiting the alternate buffer. It refactors useAlternateBuffer to read from the UI state instead of the static configuration, simplifies MouseProvider by keeping its stdin listener active for robust sequence detection, and improves terminal cleanup on exit. The review feedback correctly points out that the fallback cleanup functions in cleanupTerminalOnExit should be wrapped in a try-catch block to prevent unhandled exceptions if process.stdout is closed or in an invalid state during process termination.
| // Fallback cleanup using explicit helper functions. | ||
| disableKittyKeyboardProtocol(); | ||
| disableModifyOtherKeys(); | ||
| disableBracketedPasteMode(); | ||
| exitAlternateScreen(); | ||
| disableMouseEvents(); | ||
| enableLineWrapping(); |
There was a problem hiding this comment.
The fallback cleanup functions are executed outside of any try-catch block. If process.stdout is closed, destroyed, or in an invalid state when the process exits, calling these functions (which write to stdout) can throw unhandled exceptions (such as ERR_STREAM_WRITE_AFTER_END). Since this is an exit/cleanup handler, any unhandled exception can crash the exit process or clutter the console with stack traces.
We should wrap the fallback cleanup in a single try-catch block to ensure that all exit cleanups are safe and do not crash the process, while avoiding wrapping each individual call in its own block.
try {
// Fallback cleanup using explicit helper functions.
disableKittyKeyboardProtocol();
disableModifyOtherKeys();
disableBracketedPasteMode();
exitAlternateScreen();
disableMouseEvents();
enableLineWrapping();
} catch (e) {
debugLogger.warn('Failed to execute fallback terminal cleanup:', e);
}References
- When performing sequential cleanup operations writing to the same output stream (e.g.,
process.stdout), avoid wrapping each call in a separatetry...catchblock. If one write fails, subsequent writes to the same stream are also highly likely to fail.
gemini-cli
Bot
added
the
priority/p1
2 participants
Summary
Details
Related Issues
How to Validate
Pre-Merge Checklist