From 5c4c7139d7f467ce01834927576377cf0ecbc4a0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 8 Aug 2026 16:14:40 +0200 Subject: [PATCH] Remove the scroll calls that are now redundant Every one of these did by hand what focusing the list now does on its own: five hand-added scroll requests, and four origin resets that paired a "select the first item" with a "and show the top of the list". The scroll that the commits refresh performed when it found the selected commit at a new index goes too. It is now unconditional for a foreground refresh, and deliberately absent for a background one: when an agent commits in another window, we would rather see the new commits arrive than have the view yank itself back to the commit we had selected. The one origin reset that stays is the one in ReApplyFilter, which runs as part of a refresh and so can't rely on the refresh scrolling. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/gui/controllers/helpers/fixup_helper.go | 1 - pkg/gui/controllers/helpers/mode_helper.go | 5 --- pkg/gui/controllers/helpers/refresh_helper.go | 33 ++++--------------- .../helpers/refresh_helper_test.go | 11 +------ pkg/gui/controllers/helpers/search_helper.go | 4 ++- .../controllers/helpers/sub_commits_helper.go | 1 - pkg/gui/controllers/stash_controller.go | 1 - pkg/gui/menu_panel.go | 2 -- 8 files changed, 10 insertions(+), 48 deletions(-) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index e8fa43f2d..e998c2ad1 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -141,7 +141,6 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { } self.c.Contexts().LocalCommits.SetSelection(index) - self.c.Contexts().LocalCommits.FocusLine(true) self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) return nil }, diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index 6e5139744..68f7ea149 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -270,11 +270,6 @@ func (self *ModeHelper) changeFiltering(setFilter func(), selectCommit func()) e selectCommit() self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) - // The list we just selected in has nothing to do with the one - // that was showing, so wherever it was scrolled to says nothing - // about where the selection now is. PostRefreshUpdate leaves the - // scroll position alone, so ask for it separately. - self.c.Contexts().LocalCommits.FocusLine(true) return nil }, }) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 81eeec4d1..d076ced0a 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -851,33 +851,21 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, self.c.Model().CheckedOutBranch = "" } - scrollSelectionIntoView := false switch commitSelection { case types.SelectHeadCommit: if headCommitIdx := models.HeadCommitIdx(commits); headCommitIdx >= 0 { self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) - scrollSelectionIntoView = true } case types.KeepCommitSelectionByHash: if selectionRange != nil { - selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, selectionRange) + selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(commits, selectionRange) if found { self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode) - scrollSelectionIntoView = didMove } } case types.KeepCommitSelectionIndex: // The caller set the selection index deliberately; leave it untouched. } - - if scrollSelectionIntoView { - // Enqueued from within this bounce so it runs after refreshView's - // render below (which was enqueued first), matching the previous - // ordering where FocusLine ran after the view was re-rendered. - self.onUIThreadUnlessRepoChanged(env, func() { - self.c.Contexts().LocalCommits.FocusLine(true) - }) - } }) self.refreshView(self.c.Contexts().LocalCommits, env) @@ -889,8 +877,6 @@ type localCommitSelectionRange struct { selectedIsTODO bool rangeStartHash string rangeStartIsTODO bool - selectedIdx int - rangeStartIdx int mode traits.RangeSelectMode } @@ -909,8 +895,6 @@ func captureLocalCommitSelectionRange( selectedIsTODO: commits[selectedIdx].IsTODO(), rangeStartHash: commits[rangeStartIdx].Hash(), rangeStartIsTODO: commits[rangeStartIdx].IsTODO(), - selectedIdx: selectedIdx, - rangeStartIdx: rangeStartIdx, mode: mode, } } @@ -918,17 +902,16 @@ func captureLocalCommitSelectionRange( func findLocalCommitSelectionRange( commits []*models.Commit, selectionRange *localCommitSelectionRange, -) (int, int, bool, bool) { +) (int, int, bool) { selectedIdx, foundSelected := findCommitByHashPreferringTODOStatus( commits, selectionRange.selectedHash, selectionRange.selectedIsTODO) rangeStartIdx, foundRangeStart := findCommitByHashPreferringTODOStatus( commits, selectionRange.rangeStartHash, selectionRange.rangeStartIsTODO) if !foundSelected || !foundRangeStart { - return 0, 0, false, false + return 0, 0, false } - didMove := selectedIdx != selectionRange.selectedIdx || rangeStartIdx != selectionRange.rangeStartIdx - return selectedIdx, rangeStartIdx, didMove, true + return selectedIdx, rangeStartIdx, true } // findCommitByHashPreferringTODOStatus finds the commit with the given hash. @@ -1179,10 +1162,8 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refresh } } case types.SelectCheckedOutBranch: - // The checked-out branch is always at the top of the list. Setting - // the selection doesn't scroll the view, so also reset the origin. + // The checked-out branch is always at the top of the list. self.c.Contexts().Branches.SetSelectedLineIdx(0) - self.c.Contexts().Branches.GetView().SetOriginY(0) } // Need to re-render the commits view because the visualization of local @@ -1473,11 +1454,9 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en self.c.Model().ReflogCommits = reflogCommits self.c.Model().FilteredReflogCommits = filteredReflogCommits // Setting the selection here, in the same bounce that writes the list, - // keeps it on the UI thread and atomic with the list update. Setting the - // selection doesn't scroll the view, so also reset the origin. + // keeps it on the UI thread and atomic with the list update. if selectTopEntry { self.c.Contexts().ReflogCommits.SetSelectedLineIdx(0) - self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) } }) diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index 3a5f6ea82..d3829127a 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -28,8 +28,6 @@ func TestCaptureLocalCommitSelectionRange(t *testing.T) { expected: &localCommitSelectionRange{ selectedHash: "b", rangeStartHash: "a", - selectedIdx: 1, - rangeStartIdx: 0, mode: traits.RangeSelectModeSticky, }, }, @@ -74,15 +72,12 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { type expectation struct { selectedIdx int rangeStartIdx int - moved bool found bool } selectionRange := localCommitSelectionRange{ selectedHash: "b", rangeStartHash: "c", - selectedIdx: 1, - rangeStartIdx: 2, mode: traits.RangeSelectModeSticky, } @@ -97,7 +92,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { expected: expectation{ selectedIdx: 2, rangeStartIdx: 3, - moved: true, found: true, }, }, @@ -126,7 +120,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { expected: expectation{ selectedIdx: 2, rangeStartIdx: 3, - moved: true, found: true, }, }, @@ -139,7 +132,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { expected: expectation{ selectedIdx: 0, rangeStartIdx: 1, - moved: true, found: true, }, }, @@ -147,11 +139,10 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - selectedIdx, rangeStartIdx, moved, found := findLocalCommitSelectionRange(testCase.commits, &selectionRange) + selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(testCase.commits, &selectionRange) actual := expectation{ selectedIdx: selectedIdx, rangeStartIdx: rangeStartIdx, - moved: moved, found: found, } diff --git a/pkg/gui/controllers/helpers/search_helper.go b/pkg/gui/controllers/helpers/search_helper.go index bc0c938f9..51f510792 100644 --- a/pkg/gui/controllers/helpers/search_helper.go +++ b/pkg/gui/controllers/helpers/search_helper.go @@ -225,7 +225,6 @@ func (self *SearchHelper) OnPromptContentChanged(searchString string) { switch context := state.Context.(type) { case types.IFilterableContext: context.SetSelection(0) - context.GetView().SetOriginY(0) context.SetFilter(searchString, self.c.UserConfig().Gui.UseFuzzySearch()) self.c.PostRefreshUpdate(context) case types.ISearchableContext: @@ -241,6 +240,9 @@ func (self *SearchHelper) ReApplyFilter(context types.Context) { state := self.searchState() if context == state.Context && self.c.Context().Current().GetKey() == self.c.Contexts().Search.GetKey() { filterableContext.SetSelection(0) + // This runs as part of a refresh, and a refresh that no user action + // is behind keeps the scroll position, which would leave the view + // scrolled somewhere the filtered list no longer has anything at. filterableContext.GetView().SetOriginY(0) } filterableContext.ReApplyFilter(self.c.UserConfig().Gui.UseFuzzySearch()) diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index 7bd928826..09f32d1a9 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -66,7 +66,6 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { subCommitsContext.GetView().TitlePrefix = opts.Context.GetView().TitlePrefix self.c.PostRefreshUpdate(self.c.Contexts().SubCommits) - subCommitsContext.FocusLine(true) self.c.Context().Push(self.c.Contexts().SubCommits, types.OnFocusOpts{}) return nil diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 03011e421..7730587f4 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -251,7 +251,6 @@ func (self *StashController) handleRenameStashEntry(stashEntry *models.StashEntr return err } self.context().SetSelection(0) // Select the renamed stash - self.context().FocusLine(true) // Renaming re-creates the stash at the top, shifting the other // entries' indices; block input so that a quick next action sees // the refreshed list rather than the stale indices. diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index 23016b9a5..0ddefdbee 100644 --- a/pkg/gui/menu_panel.go +++ b/pkg/gui/menu_panel.go @@ -72,8 +72,6 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error { gui.State.Contexts.Menu.SetOnCancel(opts.OnCancel) gui.State.Contexts.Menu.SetSelection(0) - gui.Views.Menu.SetOriginY(0) - gui.Views.Menu.Title = opts.Title gui.Views.Menu.FgColor = theme.GocuiDefaultTextColor