From 29942337f55dd1b0683a981bd572b308823c76b9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:18:52 +0200 Subject: [PATCH 1/6] Add tests for renders that don't show after scrolling far down a diff If the main view is scrolled more than about 5000 lines down a diff and another commit is selected, the main view keeps showing the previous diff, as long as the new one is longer than 5000 lines as well. Similarly, if the jump-to-file menu of the commits panel is used to go to the last file of such a diff, the file's header ends up at the top of the view with blank space below the end of the diff. This happens even with gui.scrollPastBottom set to false. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../jump_to_the_last_file_of_a_long_diff.go | 53 ++++++++++++++++++ ...ther_long_diff_after_scrolling_far_down.go | 55 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 3 files changed, 110 insertions(+) create mode 100644 pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go create mode 100644 pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go diff --git a/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go b/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go new file mode 100644 index 000000000..fad3a21db --- /dev/null +++ b/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go @@ -0,0 +1,53 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var JumpToTheLastFileOfALongDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Jumping to the last file of a long diff from the commits panel doesn't scroll past the end of the diff", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 100, + Height: 20, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.ScrollPastBottom = false + }, + SetupRepo: func(shell *Shell) { + // Longer than the 5000 lines that a render reads at most before it waits for + // the user to scroll further. + lines := make([]string, 6000) + for i := range lines { + lines[i] = fmt.Sprintf("line%04d", i+1) + } + shell.CreateFileAndAdd("long.txt", strings.Join(lines, "\n")+"\n") + shell.CreateFileAndAdd("short.txt", "one\n") + shell.Commit("first commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("first commit")). + Press(keys.Universal.JumpToFile) + + t.ExpectPopup().Menu(). + Title(Equals("Jump to file in diff")). + Select(Equals("short.txt")). + Confirm() + + t.Views().Commits(). + IsFocused() + + // The view shows the last screenful of the diff: the end of long.txt's diff, + // and all of short.txt's below it. + t.Views().Main(). + /* EXPECTED: + TopVisibleLine(Equals("+line5994")) + ACTUAL: */ + TopVisibleLine(Equals("diff --git a/short.txt b/short.txt")) + }, +}) diff --git a/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go b/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go new file mode 100644 index 000000000..094cb99ba --- /dev/null +++ b/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go @@ -0,0 +1,55 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAnotherLongDiffAfterScrollingFarDown = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Selecting another commit shows its diff when the main view is scrolled far down a long diff", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 100, + Height: 20, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + // Longer than the 5000 lines that a render reads at most before it waits for + // the user to scroll further. + lines := make([]string, 6000) + for i := range lines { + lines[i] = fmt.Sprintf("line%04d", i+1) + } + long := strings.Join(lines, "\n") + "\n" + + shell.CreateFileAndAdd("first.txt", long) + shell.Commit("first commit") + shell.CreateFileAndAdd("second.txt", long) + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("second commit")). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Universal.GotoBottom). + SelectedLines(Contains("+line6000")). + Press(keys.Universal.Return) + + t.Views().Commits(). + IsFocused(). + SelectNextItem(). + SelectedLine(Contains("first commit")) + + t.Views().Main(). + /* EXPECTED: + Content(Contains("diff --git a/first.txt b/first.txt")) + ACTUAL: */ + Content(Contains("diff --git a/second.txt b/second.txt")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index aea6c4f13..76b8b3357 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -414,6 +414,7 @@ var tests = []*components.IntegrationTest{ main_view.JumpToAFileOfTheDiff, main_view.JumpToAFileOnlyOverADiff, main_view.JumpToAnotherFileScrollsToTheLeftEdge, + main_view.JumpToTheLastFileOfALongDiff, main_view.KeepAWrappedLineCoveredAcrossARerender, main_view.KeepAnEmptiedPaneEmpty, main_view.KeepBothHalvesOfAChangeSelected, @@ -493,6 +494,7 @@ var tests = []*components.IntegrationTest{ main_view.SelectionCommandTooltipsFollowTheDiff, main_view.SelectionCommandsOnlyWhereTheyApply, main_view.SelectionOverTheCustomPatch, + main_view.ShowAnotherLongDiffAfterScrollingFarDown, main_view.ShowTheTabSwitchedToLast, main_view.StageDeletedFile, main_view.StageDiffLines, From d198ce56f120ec0b52a4eb9388946422f28e6aa3 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:23:36 +0200 Subject: [PATCH 2/6] Extract how far a jump can scroll its target to the top of the view A jump in a pane that isn't focused is going to stop at the last screenful of the diff as well. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../controllers/helpers/diff_line_selection.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_selection.go b/pkg/gui/controllers/helpers/diff_line_selection.go index 4d6b26b51..4cc86d5b8 100644 --- a/pkg/gui/controllers/helpers/diff_line_selection.go +++ b/pkg/gui/controllers/helpers/diff_line_selection.go @@ -106,16 +106,21 @@ func (self *DiffLineHelper) PlaceNavigationTarget( } // scrollTargetToTop scrolls the given row of the diff to the top of the view, leaving -// the view where it is when that row is on screen already. The last screenful of the -// diff is as far as it goes, so that the view doesn't scroll past the end of what it is -// showing. +// the view where it is when that row is on screen already. func (self *DiffLineHelper) scrollTargetToTop(pane types.DiffPaneContext, target int) { - view := pane.GetView() originY, height := pane.GetViewTrait().ViewPortYBounds() if target >= originY && target < originY+height { return } - view.SetOriginY(min(target, max(0, view.ViewLinesHeight()-height))) + pane.GetView().SetOriginY(originYWithTargetAtTop(pane, target)) +} + +// originYWithTargetAtTop returns the origin that puts the given row of the diff at the +// top of the view. The last screenful of the diff is as far as it goes, so that the +// view doesn't scroll past the end of what it is showing. +func originYWithTargetAtTop(pane types.DiffPaneContext, target int) int { + _, height := pane.GetViewTrait().ViewPortYBounds() + return min(target, max(0, pane.GetView().ViewLinesHeight()-height)) } // CollapseNonStickyRange drops a range that only grows while shift is held back to a From 56b6c5af3133cfa6b24aede00291d4dd51f64c1c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:26:38 +0200 Subject: [PATCH 3/6] Stop at the end of the diff when jumping to a file from a panel With gui.scrollPastBottom set to false, picking the last file of a long diff from the jump-to-file menu of the commits panel scrolls that file to the top of the view. This leaves blank space below the end of the diff. A jump in a pane that isn't focused puts the file at the top of the view and leaves it to the layout to stop at the end of the diff. But the layout doesn't do this while the view is loading. Picking an item from the menu closes the menu first, and the commits panel starts a re-render of the diff as it gets the focus back. So the view is loading when the jump happens, and the layout corrects the position only in a later pass after the re-render has finished. For a long diff the re-render never finishes; a later commit fixes this. Clicking a file in the diffstat doesn't close a menu, so it doesn't have the problem. Stop the jump at the last screenful of the diff, as a jump in the focused pane does. Like that one, it stops there whatever gui.scrollPastBottom is set to. The diff in click_a_file_in_a_diff_stat_that_comes_first fits on screen, so the jump doesn't scroll it any more. Make it longer than the view. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/gui/controllers/helpers/diff_line_selection.go | 7 ++++--- .../click_a_file_in_a_diff_stat_that_comes_first.go | 6 +++++- .../main_view/jump_to_the_last_file_of_a_long_diff.go | 3 --- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_selection.go b/pkg/gui/controllers/helpers/diff_line_selection.go index 4cc86d5b8..ff9a9d2c9 100644 --- a/pkg/gui/controllers/helpers/diff_line_selection.go +++ b/pkg/gui/controllers/helpers/diff_line_selection.go @@ -65,8 +65,9 @@ func (self *DiffLineHelper) EstablishSelection(mainContext *context.MainContext, // PlaceNavigationTarget moves the pane's selection to the row a jump found, bringing // it on screen if it isn't already. With no selection to move — a pane that isn't -// focused, or one showing a diff with nothing selectable in it — the row goes to the -// top of the view instead, that being all a jump can do there. +// focused, or one showing a diff with nothing selectable in it — all a jump can do is +// scroll the row to the top of the view, or as close to the top as the end of the diff +// allows. // // alignTop asks for the target to become the view's top line, so that everything that // begins there is on screen. It only applies to a target the view has to scroll to: a @@ -84,7 +85,7 @@ func (self *DiffLineHelper) PlaceNavigationTarget( ) { view := pane.GetView() if !view.Highlight { - view.SetOrigin(0, target) + view.SetOrigin(0, originYWithTargetAtTop(pane, target)) return } if alignTop { diff --git a/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go b/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go index 2d459e21d..1f1f71fa1 100644 --- a/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go +++ b/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go @@ -1,6 +1,8 @@ package main_view import ( + "strings" + "github.com/jesseduffield/lazygit/pkg/config" . "github.com/jesseduffield/lazygit/pkg/integration/components" ) @@ -23,7 +25,9 @@ var ClickAFileInADiffStatThatComesFirst = NewIntegrationTest(NewIntegrationTestA SetupRepo: func(shell *Shell) { shell.CreateFileAndAdd("aaa.txt", "one\n") shell.Commit("one") - shell.CreateFileAndAdd("zzz.txt", "one\n") + // Longer than the view, so that the diff can be scrolled to put aaa.txt at the + // top. + shell.CreateFileAndAdd("zzz.txt", strings.Repeat("line\n", 40)) shell.Commit("two") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { diff --git a/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go b/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go index fad3a21db..e2d3c0745 100644 --- a/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go +++ b/pkg/integration/tests/main_view/jump_to_the_last_file_of_a_long_diff.go @@ -45,9 +45,6 @@ var JumpToTheLastFileOfALongDiff = NewIntegrationTest(NewIntegrationTestArgs{ // The view shows the last screenful of the diff: the end of long.txt's diff, // and all of short.txt's below it. t.Views().Main(). - /* EXPECTED: TopVisibleLine(Equals("+line5994")) - ACTUAL: */ - TopVisibleLine(Equals("diff --git a/short.txt b/short.txt")) }, }) From f91d20f67db71d9986b2d86cb0d6dc7bf584bc9e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:28:02 +0200 Subject: [PATCH 4/6] Don't let a render stop reading before it has filled the view If the main view is scrolled more than about 5000 lines down a diff and another commit with a diff of more than 5000 lines is selected, the main view keeps showing the previous diff. A render reads its command's output into an off-screen buffer, and shows it only once it has read enough lines to fill the view or has reached the end of the output. It then reads on as far as an accurate scrollbar needs, and waits for the user to scroll further. Both counts include the lines above the view, so that a re-render of the diff the user is scrolled into fills the view where it is. But the 5000-line cap that is meant for the scrollbar applies to the lines above the view as well. So if the view is scrolled further down than that, the render stops reading before it has filled the view, and never shows anything. The view also counts as loading until the next render starts, so the layout doesn't keep it from scrolling past the end of the diff in the meantime. This has been a problem since 9e23111172 made renders read into an off-screen buffer. Before that, the lines of a render that stopped early showed up anyway. Apply the cap to the lines for the scrollbar only. Also never ask for fewer lines than fill the view. A view no more than four lines tall needs fewer lines than that for its scrollbar, even at the top of a diff. A render of another diff shows it from the top, so it doesn't need the lines above the view at all. A later commit stops it from reading them. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/gui/view_helpers.go | 6 ++++-- .../show_another_long_diff_after_scrolling_far_down.go | 3 --- pkg/tasks/tasks.go | 5 +++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 9d70b6e4b..ed4002748 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -44,10 +44,12 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { linesToReadForAccurateScrollbar := min( // However, cap it at some arbitrary max limit, so that we don't get // performance problems for huge monitors or tiny font sizes - height*(height-1)/minScrollbarHeight+oy, 5000) + height*(height-1)/minScrollbarHeight, 5000) + oy return tasks.LinesToRead{ - Total: linesToReadForAccurateScrollbar, + // A view no more than a few lines tall needs fewer lines for its scrollbar + // than it takes to fill it, and the task must read at least those. + Total: max(linesToReadForAccurateScrollbar, linesForFirstRefresh), InitialRefreshAfter: linesForFirstRefresh, } } diff --git a/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go b/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go index 094cb99ba..2f0580663 100644 --- a/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go +++ b/pkg/integration/tests/main_view/show_another_long_diff_after_scrolling_far_down.go @@ -47,9 +47,6 @@ var ShowAnotherLongDiffAfterScrollingFarDown = NewIntegrationTest(NewIntegration SelectedLine(Contains("first commit")) t.Views().Main(). - /* EXPECTED: Content(Contains("diff --git a/first.txt b/first.txt")) - ACTUAL: */ - Content(Contains("diff --git a/second.txt b/second.txt")) }, }) diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 510fffa49..7e0c10676 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -167,6 +167,11 @@ type LinesToRead struct { // Number of lines after which we have read enough to fill the view, and can // do an initial refresh. Only set for the initial read request; -1 for // subsequent requests. + // + // Unless Total is -1, it must not be more than Total. The task shows nothing + // of what it reads until it has read this many lines or has reached the end + // of its input, so a task that stops reading before then leaves the view + // showing the previous render. InitialRefreshAfter int // Function to call after reading the lines is done From be300454b66d6b5e6f6a2da5bc14b6ee00b408c5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:28:17 +0200 Subject: [PATCH 5/6] Name the fields of LinesToRead in the task tests The request for the first lines of a render is going to say how many lines there are above the view, and the unnamed fields of these literals would all need another value for it. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/tasks/tasks_test.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index a5b195a9c..f0e9597d2 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -60,7 +60,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { return ExecCmd{Cmd: cmd}, reader } - fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{20, -1, nil}, onDone) + fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{Total: 20, InitialRefreshAfter: -1}, onDone) _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: func() { task.Done() }}) @@ -131,7 +131,7 @@ func TestNewCmdTask(t *testing.T) { return ExecCmd{Cmd: cmd}, reader } - fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{20, -1, nil}, onDone) + fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{Total: 20, InitialRefreshAfter: -1}, onDone) wg := sync.WaitGroup{} wg.Go(func() { time.Sleep(100 * time.Millisecond) @@ -196,7 +196,7 @@ func TestNewCmdTaskStoppedWhileProducingItsPrefix(t *testing.T) { } onDone, getOnDoneCallCount := getCounter() - fn := manager.NewCmdTask(start, prefix, LinesToRead{20, -1, nil}, onDone) + fn := manager.NewCmdTask(start, prefix, LinesToRead{Total: 20, InitialRefreshAfter: -1}, onDone) _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: func() { task.Done() }}) assert.False(t, started) @@ -277,7 +277,7 @@ func TestNewCmdTaskQueuedReadAtEndOfInput(t *testing.T) { // The initial request asks for far more lines than the reader has, so the // task reaches EOF while that request is still the one being served. - fn := manager.NewCmdTask(start, nil, LinesToRead{100, -1, nil}, func() {}) + fn := manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: -1}, func() {}) thenCalled := false wg := sync.WaitGroup{} @@ -326,7 +326,7 @@ func TestResetOriginSurvivesTaskReplacement(t *testing.T) { } // The first-paint point is far beyond what any of these readers yield, so // only reaching EOF paints. - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, onDone), key) } runTaskToCompletion := func(key string) { done := make(chan struct{}) @@ -381,7 +381,7 @@ func TestLoadingIndicatorOnlyTakesOverForNewContent(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, onDone), key) } // Starts a task whose command produces nothing at all, so that it is still // waiting for its first line when the loading indicator falls due. Returns @@ -467,7 +467,7 @@ func TestNewCmdTaskRestore(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 50} } - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 30}, func() { close(done) }), "cmd") <-done assert.Equal(t, 1, applyCount, "Apply should run exactly once") @@ -520,7 +520,7 @@ func TestNewCmdTaskRestoreThatFindsNothing(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 50} } - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 30}, func() { close(done) }), "cmd") <-done assert.Equal(t, 1, applyCount, "Apply should still run, to swap the render in") @@ -562,7 +562,7 @@ func TestRestoreSurvivesTaskReplacement(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, onDone), "cmd") } // The task the restore was installed for stalls before it can paint. @@ -609,7 +609,7 @@ func TestKeepScrollPositionForNextTask(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, onDone), key) } runTaskToCompletion := func(key string) { done := make(chan struct{}) @@ -670,7 +670,7 @@ func TestForgetRenderedContent(t *testing.T) { } done := make(chan struct{}) _ = manager.NewTask( - manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, func() { close(done) }), key) + manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, func() { close(done) }), key) <-done } @@ -699,37 +699,37 @@ func TestNewCmdTaskRefresh(t *testing.T) { { "total < initialRefreshAfter", 150, - LinesToRead{100, 120, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: 120}, []int{100}, }, { "total == initialRefreshAfter", 150, - LinesToRead{100, 100, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: 100}, []int{100}, }, { "total > initialRefreshAfter", 150, - LinesToRead{100, 50, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: 50}, []int{50, 100}, }, { "initialRefreshAfter == -1", 150, - LinesToRead{100, -1, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: -1}, []int{100}, }, { "totalTaskLines < initialRefreshAfter", 25, - LinesToRead{100, 50, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: 50}, []int{25}, }, { "totalTaskLines between total and initialRefreshAfter", 75, - LinesToRead{100, 50, nil}, + LinesToRead{Total: 100, InitialRefreshAfter: 50}, []int{50, 75}, }, } @@ -934,7 +934,7 @@ func TestMessageEndsTheLoadingOfTheTaskItReplaces(t *testing.T) { reservation := manager.ReserveTask() manager.StartLoading() - _ = manager.NewReservedTask(reservation, manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, nil), "cmd") + _ = manager.NewReservedTask(reservation, manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, nil), "cmd") <-stalled.blocked assert.True(t, manager.IsLoading()) @@ -973,7 +973,7 @@ func TestEarlierTaskEndingLeavesALaterTaskLoading(t *testing.T) { reservation := manager.ReserveTask() manager.StartLoading() _ = manager.NewReservedTask(reservation, - manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, func() { close(earlierDone) }), "earlier") + manager.NewCmdTask(start, nil, LinesToRead{Total: 100, InitialRefreshAfter: 50}, func() { close(earlierDone) }), "earlier") <-stalled.blocked // The later task is asked for, but not created yet, as for a render whose task From f6892ffc94771c5638c2feea21ea7e664838ef72 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 13 Oct 2026 16:31:22 +0200 Subject: [PATCH 6/6] Read the lines above the view only for a render that stays where it is A render first reads enough lines to fill the view, plus the lines above the view, so that a re-render of the diff the user is scrolled into fills the view where it is. But a render of another diff, such as the diff of another commit, is shown from the top. So if the view is scrolled far down a long diff, the next diff only shows up once the lines above the view have been read as well. For a diff of a few hundred thousand lines this takes a noticeable while, especially with a diff renderer such as delta. ViewBufferManager decides whether a render keeps the view where it is (see newContentPending). So let linesToReadFromCmdTask count the lines from the top of the view and pass on how many lines are above it, and let the task add those only if it keeps the view where it is. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/gui/view_helpers.go | 12 ++++++--- pkg/tasks/tasks.go | 30 +++++++++++++++++++++- pkg/tasks/tasks_test.go | 56 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index ed4002748..ff1cb1073 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -18,12 +18,14 @@ func (gui *Gui) resetViewOrigin(v *gocui.View) { // Returns the number of lines that we should read initially from a cmd task so // that the scrollbar has the correct size, along with the number of lines after -// which the view is filled and we can do a first refresh. +// which the view is filled and we can do a first refresh. Both count from the top +// of the view. The task adds the lines above the view if it shows its content +// where the view is (see LinesToRead.LinesAboveView). func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { height := v.InnerHeight() - oy := v.OriginY() + linesAboveView := v.OriginY() - linesForFirstRefresh := height + oy + 10 + linesForFirstRefresh := height + 10 // A search counts the matches in everything the view holds, so a re-render of a // view that is being searched is read all the way to the end (as opening the @@ -34,6 +36,7 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { return tasks.LinesToRead{ Total: -1, InitialRefreshAfter: linesForFirstRefresh, + LinesAboveView: linesAboveView, } } @@ -44,13 +47,14 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { linesToReadForAccurateScrollbar := min( // However, cap it at some arbitrary max limit, so that we don't get // performance problems for huge monitors or tiny font sizes - height*(height-1)/minScrollbarHeight, 5000) + oy + height*(height-1)/minScrollbarHeight, 5000) return tasks.LinesToRead{ // A view no more than a few lines tall needs fewer lines for its scrollbar // than it takes to fill it, and the task must read at least those. Total: max(linesToReadForAccurateScrollbar, linesForFirstRefresh), InitialRefreshAfter: linesForFirstRefresh, + LinesAboveView: linesAboveView, } } diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 7e0c10676..37f586222 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -174,10 +174,30 @@ type LinesToRead struct { // showing the previous render. InitialRefreshAfter int + // The number of lines above the view when the task was asked for. Only set + // for the initial read request, whose Total and InitialRefreshAfter don't + // count these lines. A task that shows its content where the view is reads + // them as well, so that it fills the view there. A task that shows content + // the view isn't showing yet shows it from the top and doesn't read them. + LinesAboveView int + // Function to call after reading the lines is done Then func() } +// includingLinesAboveView returns the initial read request for a task that shows its +// content where the view is rather than from the top. Such a task reads the lines +// above the view as well. +func (self LinesToRead) includingLinesAboveView() LinesToRead { + if self.Total != -1 { + self.Total += self.LinesAboveView + } + if self.InitialRefreshAfter != -1 { + self.InitialRefreshAfter += self.LinesAboveView + } + return self +} + // RenderRestore puts a view back where it was when it re-renders content the user // is already looking at, laid out differently — a different context size, whitespace // ignored, another diff renderer — instead of showing the new render from the top. @@ -452,6 +472,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix // remembered just before the re-render that led here was triggered. restore := self.getRestoreForNextTask() + // Content the view isn't showing yet is shown from the top (see + // newContentPending), and content the view is showing is shown where the + // view is. + initialRequest := linesToRead + if !self.newContentPending.Load() { + initialRequest = linesToRead.includingLinesAboveView() + } + if self.throttle.Load() { self.Log.Info("throttling task") time.Sleep(THROTTLE_TIME) @@ -804,7 +832,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix close(lineWrittenChan) }) - self.readRequests.enqueue(linesToRead) + self.readRequests.enqueue(initialRequest) <-done diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index f0e9597d2..12b1ce52d 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -687,6 +687,62 @@ func TestForgetRenderedContent(t *testing.T) { assert.Equal(t, 2, getResetOriginCallCount(), "an emptied view is shown its content afresh") } +// A render of the content the view is showing keeps the view where it is, so it reads +// the lines above the view as well, both before it first paints and in all. A render +// of content the view isn't showing yet is shown from the top, so the lines above the +// view are no use to it. Reading them would only hold up the first paint of a diff +// selected while the view is scrolled far down another one. +func TestOnlyARenderInPlaceReadsTheLinesAboveTheView(t *testing.T) { + writer := bytes.NewBuffer(nil) + linesWritten := func() int { return strings.Count(writer.String(), "\n") } + linesWrittenAtFirstPaint := -1 + + manager := NewViewBufferManager( + utils.NewDummyLog(), + writer, + func() {}, // beforeStart + func() {}, // refreshView + func() {}, // onEndOfInput + func() {}, // resetOrigin + writer.Reset, + func() { linesWrittenAtFirstPaint = linesWritten() }, + func() gocui.Task { return gocui.NewFakeTask() }, + // no UI thread in the test; run the view mutations inline + func(f func()) error { f(); return nil }, + ) + defer manager.Close() + + // Renders the given command's output, which is longer than any of the reading + // asked for here, and returns once the task has read the lines it was asked for. + render := func(key string) { + start := func() (Cmd, io.Reader) { + // not actually starting this because it's not necessary + return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 1000} + } + served := make(chan struct{}) + linesToRead := LinesToRead{ + Total: 20, + InitialRefreshAfter: 15, + LinesAboveView: 100, + Then: func() { close(served) }, + } + _ = manager.NewTask(manager.NewCmdTask(start, nil, linesToRead, nil), key) + <-served + } + + render("cmd1") + assert.Equal(t, 15, linesWrittenAtFirstPaint) + assert.Equal(t, 20, linesWritten()) + + render("cmd1") + assert.Equal(t, 115, linesWrittenAtFirstPaint) + assert.Equal(t, 120, linesWritten()) + + render("cmd2") + assert.Equal(t, 15, linesWrittenAtFirstPaint) + assert.Equal(t, 20, linesWritten()) +} + func TestNewCmdTaskRefresh(t *testing.T) { type scenario struct { name string