Skip to content

Commit f3a600e

Browse files
committed
docs: add backlog item for stale delete-only hunk-nav comments
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.
1 parent 6fd7d57 commit f3a600e

1 file changed

Lines changed: 23 additions & 0 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
worth: later
3+
where: app/ui/diffnav.go:583
4+
added: 2026-08-20
5+
---
6+
# hunk-nav comments claim delete-only hunks are skipped, but they are not
7+
8+
`moveToNextHunk` (`app/ui/diffnav.go:583`) and `moveToPrevHunk` (`:599`) both guard
9+
`firstVisibleInHunk(...) < 0` with `// skip delete-only hunks in collapsed mode`. That is not what
10+
happens. `isCollapsedHidden` (`app/ui/collapsed.go:415`) deliberately keeps a delete-only hunk's first
11+
removed line visible as the `⋯ N lines deleted` placeholder, so `firstVisibleInHunk` returns
12+
`hunkStart` for such a hunk and `]` lands on it. A mixed hunk always has a visible add, so it returns
13+
that. The `< 0` branch looks unreachable for any real hunk, which means both comments describe a case
14+
that does not occur and name the wrong reason for a guard that does.
15+
16+
The godoc added for `positionOnFirstChange` (`:906`) states the opposite and correct behavior, so the
17+
package now contradicts itself in three places. Surfaced by the revmux review of PR #329 and confirmed
18+
by codex, which also pinned the real behavior in `TestModel_CollapsedHunkNavigationDeleteOnly`.
19+
20+
Deferred rather than fixed inline because #329 was scoped to the `--start-at-change` flag and these two
21+
lines predate it. The fix is either correcting both comments to say what the guard is for, or
22+
establishing whether `< 0` is genuinely unreachable and dropping the branch. Worth deciding which
23+
before editing, since a comment that keeps a dead branch alive is the weaker of the two outcomes.

0 commit comments

Comments
 (0)