fix(display): fix clear-below logic at terminal bottom#95
Open
liamg wants to merge 2 commits intoreeflective:masterfrom
Open
fix(display): fix clear-below logic at terminal bottom#95liamg wants to merge 2 commits intoreeflective:masterfrom
liamg wants to merge 2 commits intoreeflective:masterfrom
Conversation
MoveCursorDown (CUD / \x1b[1B) is a no-op when the cursor is on the
last line of the terminal. This caused ClearScreenBelow to erase the
prompt/input line instead of clearing below it, making the prompt
invisible whenever the input line was at the bottom of the screen.
Replace MoveCursorDown(1) with NewlineReturn ("\r\n") in Refresh and
renderHelpers. Unlike CUD, \r\n scrolls the terminal when at the
bottom, ensuring the cursor always advances to a new line before
clearing.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #95 +/- ##
==========================================
- Coverage 33.03% 32.54% -0.50%
==========================================
Files 57 58 +1
Lines 9109 9207 +98
==========================================
- Hits 3009 2996 -13
- Misses 6062 6173 +111
Partials 38 38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
|
Hello ! Thanks for this. |
Contributor
Author
|
@maxlandon Yep, that's the main way I'm using it atm and it seems ok 👍 |
Member
|
Okay give me a few days and I will merge it. Very very busy at the moment... |
Contributor
Author
|
No rush at all, thank you! |
liamg
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ClearScreenBelowerasing the prompt/input line when the cursor is on the last terminal row❯ ❯) and terminal lockup when the prompt is near the bottom of the screenProblem
MoveCursorDown(CUD /\x1b[1B) is a no-op on the last terminal line. The originalRefresh()code used CUD to move below the input before callingClearScreenBelow, which meant the clear would erase the prompt itself when at the bottom.The initial fix (replacing CUD with
\r\n) introduced a regression:\r\nunconditionally scrolls the terminal when on the last row, displacing the prompt upward on every refresh cycle. This caused the last line of multiline prompts to render twice and could lead to the terminal appearing to lock up (likely due to corrupted cursor position state).Fix
Skip the clear-below entirely when the input line is already at the bottom of the terminal — there's nothing below to clear in that case. When not at the bottom, use the original CUD approach which works correctly.
Also simplify
renderHelperssinceRefresh()now handles the clear-below before calling it.I've tested this on a few terminals and it seems to work well.