feat: add --start-at-change to open files on the first changed line - #329
Merged
Conversation
By default the diff pane opens at line 1, so on a large file the reviewer scrolls to reach the change. Measured over this repo's last 60 commits, the median first changed line is 72 and about 60% of file opens show no changed line on a 40-row pane. The flag is opt-in and default off, matching how every comparable navigation knob ships here. It reuses the positioning the cross-file hunk jump already performs: applyPendingHunkJump's forward branch is extracted as positionOnFirstChange, so startup matches what next_hunk does on a fresh file and no new positioning rules are introduced. skipInitialDividers is left untouched because its other callers depend on its current meaning. The new branch in handleFileLoaded sits below the annotation-jump, hunk-jump and compact-anchor branches so an explicit target always wins, and returns early since the trailing GotoTop would otherwise undo the scroll. Files with no hunks fall back to the first visible line, so context-only sources are unaffected. In collapsed mode the cursor lands on a delete-only hunk's placeholder head rather than skipping it. Related to #325.
… test The godoc on positionOnFirstChange described the caller's centerViewportOnCursor as a duplicate render, which invited deleting it. That call is load-bearing: centerHunkInViewport sets the offset before rendering, so the offset clamps against the previously loaded file's length, and on the no-hunk path nothing renders at all. TestModel_StartAtChange_RendersTheLoadedFile pins both halves. Removing the call makes a context-only load paint a blank pane, and makes a 400-line jump clamp to offset 0 against a one-line previous file, leaving the change far below the fold. The existing subtests could not catch either, since they assert only the cursor index against a zero-height viewport that renders nothing. Related to #325.
umputun
added a commit
that referenced
this pull request
Aug 20, 2026
moveToNextHunk and moveToPrevHunk both label their firstVisibleInHunk < 0 guard as skipping delete-only hunks in collapsed mode. A delete-only hunk keeps its first removed line visible as the placeholder, so that branch looks unreachable and both comments name the wrong reason. Surfaced by the review of #329, whose new positionOnFirstChange godoc states the opposite and correct behavior.
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.
adds
--start-at-change, an opt-in flag that puts the cursor on the first changed line when a file loads instead of leaving it at line 1. Off by default, withREVDIFF_START_AT_CHANGEand astart-at-changeconfig key.the friction is measurable. Over the last 60 commits here, 373 changed files, the median first changed line is 72 and at the 90th percentile it is 591. On a 40-row pane that means roughly 60% of file opens show no changed line at all, so the reviewer scrolls or hits
]before the review starts.--compactalready solves this by shrinking the diff, but it does so by dropping the surrounding source. This keeps the full-file context and only moves the cursor.how it works
no new positioning logic.
applyPendingHunkJump's forward branch already does exactly this when]crosses into the next file, so it is extracted aspositionOnFirstChangeand reused. Startup now matches what]does on a fresh file.the new branch in
handleFileLoadedsits below the annotation-jump, hunk-jump and compact-anchor branches, so an explicit target always wins, and it returns early because the trailingGotoTopwould undo the scroll.skipInitialDividersis untouched: five other callers depend on its current meaning, including thehomekey and the fallback the #271 compact anchor relies on.edge cases:
--only,--all-filesand--stdinare unaffectedon the second commit
the first commit's godoc described the caller's
centerViewportOnCursoras a duplicate render, which invited deleting it. That call is required.centerHunkInViewportsets the offset before rendering, so the offset clamps against the previously loaded file's length, and on the no-hunk path nothing renders at all.TestModel_StartAtChange_RendersTheLoadedFilepins both halves. I removed the call to check the test catches it: a context-only load paints a blank pane, and a 400-line jump clamps to offset 0 against a one-line previous file, leaving the change far below the fold. The earlier subtests could not catch either, since they assert only the cursor index against a zero-height viewport that renders nothing.Related to #325.