From 4fe993442c3ba438810eada37e0727fe905c7e7f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 13 Sep 2026 13:49:16 +0200 Subject: [PATCH] Ask a diff where each of its files begins File navigation works out where the neighbouring file begins by walking the rows itself, forwards or, more laboriously, backwards. A menu of the diff's files needs the same rows, all of them at once. Extract fileStarts, which answers that for the whole diff, and have navigation pick its neighbour out of the answer. The two then agree on where a file begins by construction, and the walk backwards over a file goes away. Co-Authored-By: Claude Opus 5 (1M context) --- .../controllers/helpers/diff_line_queries.go | 71 +++++++++++-------- .../helpers/diff_line_queries_test.go | 35 +++++++++ 2 files changed, 75 insertions(+), 31 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_queries.go b/pkg/gui/controllers/helpers/diff_line_queries.go index 9f3a184ee..b03a98739 100644 --- a/pkg/gui/controllers/helpers/diff_line_queries.go +++ b/pkg/gui/controllers/helpers/diff_line_queries.go @@ -363,49 +363,58 @@ func (self *DiffLineHelper) filePaths(view *gocui.View) []string { return paths } -// fileStart finds, in a diff whose lines carry the file path they belong to (empty for -// a row no backend could place), the first located row of the file adjacent to `from` -// in the given direction — the row file navigation lands on. It is the pure index -// arithmetic behind AdjacentFile. +// diffFileStart is where one file of a diff begins: the path of the file, and the row +// of the diff its section starts at. +type diffFileStart struct { + path string + row int +} + +// fileStarts finds, in a diff whose lines carry the file path they belong to (empty for +// a row no backend could place), where each file of it begins, in the order the diff +// shows them. // -// A file is identified by its path, so we look for where the path changes, skipping -// rows that carry none: those are the blank separator rows between files, or the -// header rows of a diff renderer that doesn't state which file its headers belong to. -// So the landing row is the file's header wherever the source says so — a parseable -// buffer, or a renderer that tags its headers — and the file's first content line -// otherwise, which is an accepted degradation. +// A file is identified by its path, and the rows showing it are consecutive, so a path +// differing from the one before it begins a file. Rows carrying no path are passed over: +// those are the blank separator rows between files, or the header rows of a diff +// renderer that doesn't state which file its headers belong to. So a file begins at its +// header wherever the source says so (a parseable buffer, or a renderer that tags its +// headers), and at its first content line otherwise, which is an accepted degradation. +func fileStarts(paths []string) []diffFileStart { + starts := []diffFileStart{} + previousPath := "" + for row, path := range paths { + if path == "" || path == previousPath { + continue + } + previousPath = path + starts = append(starts, diffFileStart{path: path, row: row}) + } + return starts +} + +// fileStart returns where the file adjacent to `from` in the given direction begins — +// the row file navigation lands on. It is the pure index arithmetic behind AdjacentFile. +// ok is false at the first or last file of the diff. func fileStart(paths []string, from int, forward bool) (int, bool) { anchorPath, ok := anchorFilePath(paths, from) if !ok { return 0, false } - if forward { - for i := from; i < len(paths); i++ { - if paths[i] != "" && paths[i] != anchorPath { - return i, true - } - } + starts := fileStarts(paths) + _, anchor, ok := lo.FindIndexOf(starts, func(start diffFileStart) bool { + return start.path == anchorPath + }) + if !ok { return 0, false } - // Walk back past the current file (its rows and any unlocated ones) to the previous - // file's last located row, then back over that whole file, landing on its first. - i := from - for i >= 0 && (paths[i] == "" || paths[i] == anchorPath) { - i-- - } - if i < 0 { + target := anchor + lo.Ternary(forward, 1, -1) + if target < 0 || target >= len(starts) { return 0, false } - prevPath := paths[i] - for i > 0 && (paths[i-1] == "" || paths[i-1] == prevPath) { - i-- - } - for paths[i] != prevPath { - i++ - } - return i, true + return starts[target].row, true } // anchorFilePath returns the path of the file the anchor sits in: the first row at or diff --git a/pkg/gui/controllers/helpers/diff_line_queries_test.go b/pkg/gui/controllers/helpers/diff_line_queries_test.go index 47769ad98..25d7a5433 100644 --- a/pkg/gui/controllers/helpers/diff_line_queries_test.go +++ b/pkg/gui/controllers/helpers/diff_line_queries_test.go @@ -48,6 +48,41 @@ func TestChangeBlockStart(t *testing.T) { } } +func TestFileStarts(t *testing.T) { + scenarios := []struct { + name string + paths []string + expected []diffFileStart + }{ + { + name: "a parseable diff begins each file at its header", + paths: []string{"a", "a", "a", "b", "b"}, + expected: []diffFileStart{{path: "a", row: 0}, {path: "b", row: 3}}, + }, + { + name: "a diff whose headers carry no path begins each file at its first content line", + paths: []string{"", "", "a", "a", "", "", "b", "b"}, + expected: []diffFileStart{{path: "a", row: 2}, {path: "b", row: 6}}, + }, + { + name: "an unlocated row within a file doesn't begin another one", + paths: []string{"a", "", "a", "b"}, + expected: []diffFileStart{{path: "a", row: 0}, {path: "b", row: 3}}, + }, + { + name: "a diff with no located rows shows no files", + paths: []string{"", ""}, + expected: []diffFileStart{}, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, fileStarts(s.paths)) + }) + } +} + func TestFileStart(t *testing.T) { // A parseable two-file diff: every row carries its file's path, headers included, // as the buffer parser reports it.