fix(editor): PageDown advances on consecutive long wrapped lines#7555
Open
JohnMcLear wants to merge 1 commit intoether:developfrom
Open
fix(editor): PageDown advances on consecutive long wrapped lines#7555JohnMcLear wants to merge 1 commit intoether:developfrom
JohnMcLear wants to merge 1 commit intoether:developfrom
Conversation
…lines The page up/down handler advances the caret by numberOfLinesInViewport computed from scroll.getVisibleLineRange(). That helper returns indices into rep.lines (logical lines, not visual/wrapped rows), so when one wrapped logical line fills the viewport — e.g., three consecutive lines of ~2000 chars each — the range collapses to [n, n] and the advance count becomes 0. The caret stays on line n, scroll stays at 0, and the user sees "PageDown does nothing". Clamp the advance to at least one logical line so the caret and viewport always move. Includes a Playwright regression test covering the reporter's repro (three very long lines, Ctrl+Home, PageDown). Closes ether#4562 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review Summary by QodoFix PageDown/PageUp advance on consecutive long wrapped lines
WalkthroughsDescription• Fix PageDown/PageUp becoming no-ops on consecutive long wrapped lines • Clamp viewport line count to minimum of 1 logical line • Add regression test for issue #4562 with very long wrapped lines Diagramflowchart LR
A["getVisibleLineRange returns logical line indices"] -->|"collapses to [n,n] for wrapped lines"| B["numberOfLinesInViewport = 0"]
B -->|"causes no-op"| C["PageDown/PageUp fails"]
D["Math.max clamp to 1"] -->|"guarantees minimum advance"| E["PageDown/PageUp always moves"]
File Changes1. src/static/js/ace2_inner.ts
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
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
Fix for #4562 — PageDown (and PageUp) became no-ops when the cursor sat on a very long wrapped line and the following lines were also very long.
Root cause
The page up/down handler in
ace2_inner.tsadvances the caret bynumberOfLinesInViewport, computed fromscroll.getVisibleLineRange(rep). That helper returns indices intorep.lines— logical lines, not visual/wrapped rows. When a single wrapped logical line fills the viewport (e.g., three ~2000-char lines), the range collapses to[n, n]and the advance count becomes0:Fix: clamp to at least one logical line so the caret and viewport always advance.
Test plan
Ctrl+Home,PageDown→ viewport scrolls or caret advances)page_up_down.spec.tsstill passes (no behavior change for the common case where the viewport spans multiple logical lines)pnpm run ts-checkclean locallyCloses #4562
🤖 Generated with Claude Code