fix(ui): bound horizontal scroll to the widest rendered line - #337
Merged
Conversation
Holding the right arrow scrolled scrollX past the widest line without limit. Once past every line applyHorizontalScroll takes its plain-cut branch with both overflow flags false, so every row cuts to nothing and the diff pane goes blank with no « indicator to explain it — recoverable only by scrolling all the way back or switching files. Every nonzero write to scrollX now goes through setScrollX, bounded by the widest row the pane currently renders, so a runaway offset can no longer cut the whole document away. This does not promise every viewport shows content at every legal offset: a wide row further down the file owns the bound, so short rows on screen can still cut to empty. That per-line behavior lives in applyHorizontalScroll and is pinned as deliberate by TestModel_ApplyHorizontalScrollNoLeftIndicatorWhenScrolledPastContent. The bound reads the rendered document rather than the loaded one: collapsed mode hides removed lines, so a wide hidden removal must not widen it, and expanding a hunk reveals those lines again. Delete-only placeholders are measured from their own synthetic text. Per-line widths are cached at load, parallel to file.lines, so the bound costs an integer scan rather than re-measuring strings on every keypress. The walk uses a moving hunk index rather than hunkStartFor, which rescans the whole hunk slice per line. Layout changes that widen the pane lower the bound without a horizontal keypress of their own. All four — resize, tree hide, line-number and blame toggles — already render through syncViewportToCursor, so the clamp lives there; the two collapsed mutations render directly and clamp locally. Related to #334, which surfaced this while asking for mouse horizontal scroll. The mouse gesture is a separate question and stays open.
umputun
added a commit
that referenced
this pull request
Aug 28, 2026
The bound added in #337 is document-wide, so a wide row anywhere in the file sets the ceiling. Scroll toward it while only short rows are visible and every visible row cuts to empty with no « indicator. Filed as maybe rather than later: a per-viewport bound would keep the pane populated but break column alignment between rows, and drawing the indicator on a row scrolled past its own end changes behavior a test currently pins. Nobody has ruled on either.
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.
holding the right arrow scrolled
layout.scrollXpast the widest line without limit. Once the offset passed the document's widest row,applyHorizontalScrolltakes its plain-cut branch with both overflow flags false, so every row cuts to nothing and the diff pane goes blank with no«to explain it.what changed
every nonzero write to
scrollXnow goes throughsetScrollX, bounded by the widest horizontally scrollable row in the current rendered document, so a runaway offset can no longer cut the whole document away.the bound reads the rendered document rather than the loaded one. Collapsed mode hides removed lines, so a wide hidden removal must not widen it, and expanding a hunk reveals those lines again. Delete-only placeholders are measured from their own synthetic text.
per-line widths are cached at load, parallel to
file.lines, so the bound costs an integer scan rather than re-measuring strings on every keypress.layout changes that widen the pane lower the bound without a horizontal keypress of their own. All four (resize, tree hide, line-number and blame toggles) already render through
syncViewportToCursor, so the clamp lives there. The two collapsed mutations render directly and clamp locally.what this does not fix
the bound is document-wide, so a wide row further down the file owns it and short rows on screen can still cut to empty. The renderer omits the
«indicator in that case, pinned byTestModel_ApplyHorizontalScrollNoLeftIndicatorWhenScrolledPastContent. Changing that requires a separate product choice, because document-wide offsets keep columns aligned across rows.tests
tests fail if any
clampHorizontalScrollorsetScrollXcall is removed. Coverage forapp/uiis 95.7%.related to #334, which surfaced this while asking for mouse horizontal scroll. This adds no mouse support; that gesture is a separate question and #334 stays open.