feat: decouple mouse wheel from diff cursor#173
Merged
Conversation
wheel events on the diff pane now scroll the viewport only. the diff cursor stays on its current logical line and is pinned to the visible edge if scrolling pushes it off-screen. matches less/vim mouse semantics. before, every wheel notch advanced the cursor by the wheel step and dragged the viewport along to keep the cursor row stable. under trackpad momentum that felt automatic and slightly inertial because each event re-rendered the diff to move the cursor highlight. the new path skips the re-render when the cursor stays in place, so plain scrolling is just a viewport offset change.
There was a problem hiding this comment.
Pull request overview
This PR changes mouse-wheel behavior in the diff pane so wheel events scroll the diff viewport without advancing the diff cursor, reducing “runaway” cursor movement on trackpad inertial/momentum scrolling and aligning behavior with less/vim-style mouse panning.
Changes:
- Update diff-pane wheel handling to scroll
viewport.YOffsetand pin the diff cursor to the visible edge only if it would otherwise go off-screen. - Add/update mouse interaction tests to validate viewport scrolling, cursor pinning, no-op behavior when content fits, and TOC active-section sync.
- Update user documentation (README and the two reference copies) to describe the new wheel semantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents that diff-pane wheel scrolls viewport only and pins cursor when pushed off-screen. |
| app/ui/mouse.go | Reworks diff-pane wheel handling via scrollDiffViewportBy + pinDiffCursorTo. |
| app/ui/mouse_test.go | Updates/adds tests for viewport-only scrolling, cursor pinning, and TOC sync. |
| plugins/codex/skills/revdiff/references/usage.md | Mirrors README mouse-wheel semantics for plugin references. |
| .claude-plugin/skills/revdiff/references/usage.md | Mirrors README mouse-wheel semantics for plugin references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+258
to
+289
| func (m *Model) scrollDiffViewportBy(delta int) { | ||
| if m.file.name == "" { | ||
| return | ||
| } | ||
| maxOffset := max(0, m.layout.viewport.TotalLineCount()-m.layout.viewport.Height) | ||
| current := m.layout.viewport.YOffset | ||
| target := max(0, min(current+delta, maxOffset)) | ||
| if target == current { | ||
| return | ||
| } | ||
| cursorMoved := m.pinDiffCursorTo(target) | ||
| if cursorMoved { | ||
| m.layout.viewport.SetContent(m.renderDiff()) | ||
| } | ||
| m.layout.viewport.SetYOffset(target) | ||
| } | ||
|
|
||
| // pinDiffCursorTo clamps the diff cursor so its visual range overlaps the | ||
| // viewport range that would be visible at newOffset. when the cursor's | ||
| // visual range still overlaps the viewport, it is left alone. when the | ||
| // cursor sits above the viewport, it is pinned to the topmost visible row; | ||
| // when below, to the bottommost visible row. returns true when the cursor | ||
| // actually changed position (idx or annotation flag), so callers know | ||
| // whether a re-render is required. | ||
| func (m *Model) pinDiffCursorTo(newOffset int) bool { | ||
| if len(m.file.lines) == 0 { | ||
| return false | ||
| } | ||
| cursorTop, cursorBottom := m.cursorVisualRange() | ||
| viewTop := newOffset | ||
| viewBottom := newOffset + m.layout.viewport.Height - 1 | ||
| if cursorBottom >= viewTop && cursorTop <= viewBottom { |
trim trailing newline in config_test.go and re-align struct field paddings in compare_test.go to match gofmt output. no behavior change.
- [major] app/ui/mouse.go:pinDiffCursorTo: fix cursor-invisible bug in wrap mode — overlap check now uses cursorTop (marker row) instead of cursorBottom; when the cursor line straddles the viewport top boundary, advance past the continuation rows using cursorBottom+1 so the next line marker is actually visible - [major] site/docs.html:560: sync scroll-wheel description to match README and plugin reference docs (viewport-only behavior with cursor pinning) - [minor] app/ui/mouse.go:scrollDiffViewportBy: fix godoc - "clamp boundary" replaced with accurate "clamped target equals current offset" - [minor] app/ui/mouse_test.go: add WheelUp_PinsCursorToBottom (bottom-pin branch), WheelDown_NoopAtMaxOffset (maxOffset no-op), WheelInWrapMode_PinsToFirstVisibleLine (wrap-mode regression guard)
- [test-gap] app/ui/mouse_test.go: replace weak WheelInWrapMode_PinsToFirstVisibleLine test (did not exercise straddle branch) with two focused tests: WheelInWrapMode_CursorAboveViewport (cursor entirely above viewport in wrap mode) and WheelInWrapMode_StraddlePinsToNextLine (cursorTop < viewTop <= cursorBottom straddle branch, verifies cursorBottom+1 advance to next line)
Deploying revdiff with
|
| Latest commit: |
a1fef33
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2cff4a87.revdiff.pages.dev |
| Branch Preview URL: | https://feat-wheel-scroll-viewport-o.revdiff.pages.dev |
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.
scrolling the diff with a trackpad felt inertial and hard to stop. Each wheel notch moved the cursor (not just the viewport), and the viewport was dragged along to keep the cursor row stable. Under trackpad momentum the terminal emits a burst of wheel events for one swipe, so cursor and view kept marching after the finger lifted. The scroll felt automatic and cumulative.
wheel events in the diff pane now scroll the viewport only. The cursor stays on its current logical line; if scrolling pushes it off-screen, it gets pinned to the topmost or bottommost visible row so the highlight stays on screen. This matches the less/vim mouse model: wheel pans the view, the cursor is just a saved logical position.
implementation
handleWheelforhitDiffcalls a newscrollDiffViewportBy(delta)helper that clamps againstTotalLineCount-Height, callspinDiffCursorTo(newOffset)to clamp the cursor when its visual range falls outside the new viewport range, then setsYOffset. Content is re-rendered only when the cursor actually moves.tests
updated/added in
app/ui/mouse_test.go:WheelInDiffasserts viewport scroll + cursor pinWheelInDiff_CursorStaysWhenInViewfor the in-view caseWheelInDiff_NoopWhenContentFitsfor short diffsShiftWheelHalfPageasserts new YOffset semanticsWheelInDiffSyncsTOCActiveSectionuses long enough content for the viewport to actually scrolldocs
README.mdand both plugin reference copies (.claude-plugin/...andplugins/codex/..., byte-identical) updated to describe the new wheel semantics.