fix(pager): G/End overshoot makes k/Up unresponsive after jump to bottom#1706
Closed
wlon wants to merge 2 commits into
Closed
fix(pager): G/End overshoot makes k/Up unresponsive after jump to bottom#1706wlon wants to merge 2 commits into
wlon wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request modifies the max_scroll calculation in the TUI pager to prevent over-scrolling when jumping to the bottom, ensuring that subsequent scroll-up actions are immediately visible. It also adds unit tests to verify this behavior. A review comment suggests removing a redundant .max(1) call from the new calculation to simplify the code.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Hmbown
added a commit
that referenced
this pull request
May 17, 2026
Bump workspace, inter-crate, and npm package versions 0.8.38 -> 0.8.39. Roll CHANGELOG [Unreleased] into [0.8.39] with all fixes: - Revert v0.8.38 /model picker rework (back to instant curated picker) - Restore approval grouping (lossy v0.8.37 logic for approvals, exact key for denials) - Thinking-only turn surface fix (#1727) - ACP server JSON-RPC id stringification (#1696) - Chat client: reasoning_content for generic providers (#1673) - Compaction: user text query preservation (#1704) - Engine: system prompt override survival (#1688) - Pager: G/End overshoot fix (#1706), mouse scroll (#1716) - Composer: scroll with text (#1677), multiline arrows (#1721) - macOS system theme detection (#1670) - rlm_open blank source fields (#1712) - Terminal resize paging fix (#1724) - Docker first-run permission (#1684) - README Rust 1.88+ requirement note (#1718) Tests: 3149 passed, 0 failed (deepseek-tui crate) clippy: clean on --all-targets --all-features
Owner
|
Thanks for the clear pager repro. The G/End bottom clamp fix was harvested into v0.8.39 via #1734 and credited in the changelog, so I am closing this PR to keep the queue clean. Please reopen if v0.8.39 still needs multiple k/Up presses after jumping to the bottom. |
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
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
After pressing
GorEndto jump to the bottom of the pager (Ctrl+O reasoning list, tool detail, etc.), pressingk/Upto scroll back up had no visible effect for many consecutive presses.Root cause
max_scroll()returnedlines.len() - 1whilerender()clamped tolines.len() - visible_height. The gap meantGsetscrollfar above the render clamp. Eachk/Uppress decrementedscrollinside that overshoot gap, producing zero visible movement until the value finally fell below the clamp.For a 50-line pager with ~18 visible lines, this required 17 consecutive
kpresses before any content scrolled up.Fix
Changed
max_scroll()to usepage_height()(the cachedvisible_heightfrom the last render), matching the render-side clamp:Tests
Two regression tests added. All 33 pager tests pass.