fix: single-line files — decorate the visible wrapped rows, and stop the wheel snapping to a 100 KB boundary - #2846
Merged
Conversation
Opening the 441 KB single-line JSON from fresh#2838, clicking the scrollbar to jump partway down, then scrolling the wheel threw the scrollbar position away: the first notch snapped top_byte to exactly 100,000 and reset the row offset, after which top_byte was frozen and each notch advanced only three rows. The view jumped back toward the top of the file and then barely moved. 100,000 is MAX_LINE_BYTES, the budget LineIterator uses to avoid materialising a huge line in one go — it splits such a line into pieces and yields each as a separate "line". That is a read budget, not document structure, but the wrapped-scroll math consumed it as though a piece boundary were a line boundary: it counted the visual rows of a 100 KB piece instead of the whole line, ran out of rows inside it, and "advanced" to the next piece as if it were the next line. The scrollbar, which reads VisualRowIndex (real logical lines), disagreed — hence the jump on the first notch after a jump. Add LineIterator::next_logical_line, which rejoins the pieces up to the real terminator, and read through it from every wrapped-scroll path (scroll up/down, the scroll limit, the max-scroll search, the clamp, and cursor-row math). next_line keeps its budget for callers that stream. The per-line row-count cache made this stick even after the traversal was fixed. It is keyed by line_start, so the piece-based caller had stored "916 rows" under the key for a line that is really 4130 rows, and every later lookup — wheel, scrollbar clamp, ensure-visible — read that back. Gate the cache on the text actually covering the line, so a partial-line caller (the cursor-row prefix math also passes one) can no longer poison it. Fixes part 2 of #2843. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F54wthCxJtDmeUmgmnZzoU
Scrolling into the 441 KB single-line JSON from fresh#2838 lost syntax highlighting: past the first few wrapped rows the whole screen rendered in one colour, and further down in no colour at all. Byte ranges the highlighter had perfectly good spans for were drawn undecorated. The decoration pass asks for a byte window built from top_byte and calculate_viewport_end, which walks logical lines from top_byte and clamps each to one screen row's worth of columns. That is the right model for horizontal scrolling, where a long line shows one row's window of itself, and it is what bounds the per-frame scan behind fresh#2529. Under soft wrap neither half holds: the drawn rows can all belong to one logical line, and the top row can sit thousands of wrap segments into it. On the issue's file the request stayed at 0..952 no matter how far down the user scrolled. Whatever single span overlapped that window — the enclosing JSON string, or on the reporter's setup a Comment span — was returned in full and painted every row below it; once no span overlapped at all, the rows were left bare. Under wrap, derive the window from the view lines about to be rendered: they already carry a source byte per character, so the span is exact and costs a walk proportional to the screen rather than to the line. The unwrapped path keeps calculate_viewport_end unchanged. This also gives the overlay, bracket and diagnostic passes — which take the same window — the range the user is actually looking at. Fixes part 1 of #2843. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F54wthCxJtDmeUmgmnZzoU
The wrapped-line highlighting test jumped to the *bottom* of the scrollbar track and looked for the line's tail, and the tail never appeared: at max scroll the scrollbar's VisualRowIndex counts more wrap rows than the renderer builds (they derive the gutter width differently), so the requested row offset is past the last view line and the render falls back to the top of the line. That is a pre-existing disagreement in the same area as #1574 / #2649, not something this branch changes — but it made the test fail on its precondition instead of exercising the fix. Land two thirds down the track instead, where both agree, and drop the dependence on any one position: the line is now ascending "M######":424242 pairs, so every wrapped row carries both a string and a number and the assertion works wherever the jump lands. The marker read off the screen doubles as the precondition — if the view were still at the top of the line, where highlighting worked even before the fix, the test says so rather than passing vacuously. Verified failing with the decoration-window fix reverted (key and value both render undecorated white) and passing with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F54wthCxJtDmeUmgmnZzoU
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.
Fixes both defects in #2843, on top of #2841. Two independent fixes, one commit each.
1. Highlighting turns uniform after the first few wrapped rows
The decoration pass asks for a byte window built from
top_byteandcalculate_viewport_end, which walks logical lines fromtop_byteand clamps each to one screen row's worth of columns. Correct for horizontal scrolling — and it is what bounds the per-frame scan behind #2529 — but under soft wrap neither half holds: the drawn rows can all belong to one logical line, and the top row can sit thousands of wrap segments into it.Instrumenting the call confirms it: on the issue's
fp4_poll.jsonthe request stays at0..952no matter how far down you scroll. Whatever single span overlapped that window was returned in full and painted every row below it; once nothing overlapped, the rows were left bare.Under wrap, the window is now derived from the view lines about to be rendered — they already carry a source byte per character, so the span is exact and costs a walk proportional to the screen, not to the line. The unwrapped path keeps
calculate_viewport_endunchanged. The overlay, bracket and diagnostic passes take the same window and get the same correction.Note on the issue's leading hypothesis — it does not hold up, and no fix was needed there. Feeding the whole 441 KB line to
ParseState::parse_linein one call keeps the scope stack atstring.quoted.double.jsonat byte 43,059, 50,000 and 200,000;TextMateEngine::highlight_viewportreturnsString@49994..50005for the50,000..54,000window. syntect never leaves the string state, soparse_line_into_spansis not at fault and nothing needed chunking or state carry-over. The reportedComment@43059..62192was a rendered symptom of the window bug above: one span overlapping0..952gets returned whole and colours everything below it.2. Wheel scroll collapses to a chunk boundary
MAX_LINE_BYTES(100,000) is the budgetLineIteratoruses to avoid materialising a huge line at once — it splits such a line into pieces and yields each as a separate "line". That is a read budget, not document structure, but the wrapped-scroll math consumed it as a line boundary: it counted the visual rows of a 100 KB piece, ran out of rows inside it, and "advanced" to the next piece as if it were the next line. The scrollbar readsVisualRowIndex(real logical lines) and disagreed — hence the jump on the first notch after a scrollbar jump.LineIterator::next_logical_linerejoins the pieces up to the real terminator; every wrapped-scroll path now reads through it (scroll up/down, the scroll limit, the max-scroll search, the clamp, cursor-row math).next_linekeeps its budget for callers that stream.The per-line row-count cache made this stick even after the traversal was fixed. It is keyed by
line_start, so the piece-based caller had stored 916 rows under the key for a line that is really 4130 rows, and every later lookup — wheel, scrollbar clamp, ensure-visible — read that back. The cache is now gated on the text actually covering the line, so a partial-line caller (the cursor-row prefix math passes one too) cannot poison it.Measured on the issue's file, 120×30:
top_byte=0 offset=1642top_byte=0 offset=1642top_byte=100000 offset=3top_byte=0 offset=1645top_byte=100000 offset=18top_byte=0 offset=1660Tests
tests/e2e/issue_2843_single_line_viewport.rs— two e2e tests, both driving mouse only and asserting on rendered output:wheel_after_scrollbar_jump_continues_from_the_jump_positionreads the topmost ascending marker off the screen before and after one wheel notch.wrapped_long_line_keeps_syntax_colours_when_scrolled_intojumps two thirds down the track into a line of ascending"M######":424242pairs and asserts the visible key and value still render in different colours. The marker read off the screen doubles as the precondition, so a jump that left the view at the top of the line (where highlighting worked even before the fix) fails loudly instead of passing vacuously.line_iterator.rs— two unit tests fornext_logical_line(spans chunk boundaries and lands on the next line's start; unterminated tail). Not screen-visible, so unit-level per CONTRIBUTING §2.Verification
cargo fmt,cargo clippy --all-targets(no new warnings in the touched files) andcargo check --all-targetsare clean; the full lib suite passes (3134 tests). Both e2e tests pass locally, and the highlighting one was verified failing with the decoration-window fix reverted (key and value both render undecorated white) and passing with it.Both bugs were also reproduced, and both fixes confirmed, against the real
fp4_poll.jsonthrough the editor harness before any test was written.An adjacent defect this branch does not fix
The first revision of the highlighting test jumped to the bottom of the scrollbar track, and the tail never appeared. Cause: at max scroll the scrollbar's
VisualRowIndexcounts more wrap rows than the renderer builds — they derive the gutter width differently — so the requested row offset lands past the last view line andrender_buffersilently falls back to the top of the line. Reproducible onmaster, same area as #1574 / #2649, and out of scope here; the test now lands where the two agree. Worth a separate issue.🤖 Generated with Claude Code
https://claude.ai/code/session_01F54wthCxJtDmeUmgmnZzoU