From fdb16a1c7e00eed3e0cdf7a1e329280f6e106fb5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 23 Jun 2026 15:59:43 +0200 Subject: [PATCH] Preserve the focused main view's selection across commit rewrites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropping a hunk with `d` re-establishes the focused main view's selection on the next surviving change, which feels great. But other operations that rewrite the commit under the focused main view — moving a custom patch out into the index, undoing right after a discard or a patch move — don't run through the focused-main-view action handlers, so nothing was preserving the selection. The stale gocui selection was left painted over the new content, often as a large, now-meaningless range. Rather than teach every such command to capture and restore the selection (move-patch, undo, redo, and any future one), the focused main view now preserves it itself, by its change-line ordinal, as the diff re-renders — the command-agnostic counterpart of revealSelectionAfterPrimaryAction. The diff side panels call it from their render-to-main before triggering the render, so the restore rides the re-render. It stands down unless the focused main view is current and shows a selection, no precise restore is already pending (escape / post-stage reveal / context-size place the selection more precisely), and the diff command is actually changing. That last gate matters: a plain background refresh re-renders the same commit's diff unchanged, and there the selection — range and all — must be left alone; only a command change (e.g. a rebase rewriting the commit's hash) means the content moved. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/commits_files_controller.go | 4 + .../controllers/local_commits_controller.go | 5 ++ pkg/gui/controllers/main_view_controller.go | 61 +++++++++++++ pkg/gui/controllers/stash_controller.go | 4 + pkg/gui/controllers/sub_commits_controller.go | 4 + ...ection_after_moving_patch_out_main_view.go | 69 +++++++++++++++ pkg/integration/tests/test_list.go | 2 + .../undo_keeps_focused_main_view_selection.go | 86 +++++++++++++++++++ 8 files changed, 235 insertions(+) create mode 100644 pkg/integration/tests/patch_building/keep_selection_after_moving_patch_out_main_view.go create mode 100644 pkg/integration/tests/undo/undo_keeps_focused_main_view_selection.go diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index c303933c1..51843c1b6 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -186,6 +186,10 @@ func (self *CommitFilesController) GetOnRenderToMain() func() { // recomputed over the current content stay valid through the swap. self.c.Helpers().Staging.RefreshInclusionGutter() + // Preserve the focused-main-view selection across a commit rewrite. See + // LocalCommitsController.GetOnRenderToMain. + preserveFocusedMainViewSelectionAcrossContentChange(self.c, task) + self.c.RenderToMainViews(types.RefreshMainOpts{ Pair: self.c.MainViewPairs().Normal, Main: &types.ViewUpdateOpts{ diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 40c88fa46..1aee0b206 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -713,6 +713,11 @@ func (self *LocalCommitsController) GetOnRenderToMain() func() { // the marks recomputed over the current content stay valid through the swap. self.c.Helpers().Staging.RefreshInclusionGutter() + // If this re-render is a commit rewrite under the focused main view (drop, + // move-patch-out, undo, …), keep the selection on a surviving change rather + // than leaving the stale range painted. + preserveFocusedMainViewSelectionAcrossContentChange(self.c, task) + self.c.RenderToMainViews(types.RefreshMainOpts{ Pair: self.c.MainViewPairs().Normal, Main: &types.ViewUpdateOpts{ diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 8d31bd882..77a3b697c 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -513,6 +513,67 @@ func revealSelectionAfterPrimaryAction(c *ControllerCommon, sourceViewName strin }) } +// preserveFocusedMainViewSelectionAcrossContentChange keeps the focused main view's +// selection sensible when its diff is about to re-render with *different* content — e.g. a +// commit's diff shrinking after the custom patch built from it is moved into the index, or +// the whole diff changing after an undo. Those mutations don't run through the +// focused-main-view action handlers (which install their own reveal; see +// revealSelectionAfterPrimaryAction), so without this the stale selection — often a large, +// now-meaningless range — would be left painted over the new content. +// +// It is the command-agnostic counterpart of revealSelectionAfterPrimaryAction: rather than +// teaching every mutating command to capture the selection, the focused main view preserves +// it itself, by its change-line ordinal, as the diff re-renders. Call it from a diff side +// panel's render-to-main with the task about to render into the main pane, before +// triggering the render so the restore rides it. +// +// It stands down — leaving the selection exactly as is — unless all of these hold, so it +// neither fights a more precise restore nor disturbs the selection needlessly: +// - the focused main view is the current context and shows a selection; +// - no precise restore is already pending (escape / post-stage reveal / context-size), +// which places the selection more precisely than an ordinal can; +// - the diff command is changing. A plain refresh re-renders the *same* commit's diff, +// identical content under an identical command, and there the selection — range and +// all — must be left alone; only a command change (e.g. a rebase rewriting the commit's +// hash) means the content, and so the old selection's position, actually moved. +func preserveFocusedMainViewSelectionAcrossContentChange(c *ControllerCommon, mainTask types.UpdateTask) { + mainContext := c.Contexts().Normal + if c.Context().Current() != mainContext { + return + } + view := mainContext.GetView() + if !view.Highlight { + return + } + manager := c.GetViewBufferManagerForView(view) + if manager == nil || manager.GetRestoreForNextTask() != nil { + return + } + newKey, ok := diffTaskCommandKey(mainTask) + if !ok || newKey == manager.GetTaskKey() { + return + } + + // Anchor on the selection's first line, matching every revealSelectionAfterPrimaryAction + // caller, so a range collapses to its top and the ordinal lands on the nearest surviving + // change there. + first, _ := view.SelectedLineRange() + revealSelectionAfterPrimaryAction(c, mainContext.GetViewName(), mainContext.GetViewName(), first) +} + +// diffTaskCommandKey returns the buffer-manager task key a diff render task will register +// under — the joined command line, as newCmdTask/newPtyTask derive it — or false for a +// non-command task (a placeholder string), which has no command to compare. +func diffTaskCommandKey(task types.UpdateTask) (string, bool) { + switch t := task.(type) { + case *types.RunCommandTask: + return strings.Join(t.Cmd.Args, " "), true + case *types.RunPtyTask: + return strings.Join(t.Cmd.Args, " "), true + } + return "", false +} + func (self *MainViewController) enter() error { if !self.context.GetView().Highlight { return nil diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index adcaebea5..f6a849b60 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -106,6 +106,10 @@ func (self *StashController) GetOnRenderToMain() func() { // built from this panel. See LocalCommitsController.GetOnRenderToMain. self.c.Helpers().Staging.RefreshInclusionGutter() + // Preserve the focused-main-view selection across a content change. See + // LocalCommitsController.GetOnRenderToMain. + preserveFocusedMainViewSelectionAcrossContentChange(self.c, task) + self.c.RenderToMainViews(types.RefreshMainOpts{ Pair: self.c.MainViewPairs().Normal, Main: &types.ViewUpdateOpts{ diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index 131047597..48f026ed4 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -55,6 +55,10 @@ func (self *SubCommitsController) GetOnRenderToMain() func() { // built from this panel. See LocalCommitsController.GetOnRenderToMain. self.c.Helpers().Staging.RefreshInclusionGutter() + // Preserve the focused-main-view selection across a commit rewrite. See + // LocalCommitsController.GetOnRenderToMain. + preserveFocusedMainViewSelectionAcrossContentChange(self.c, task) + self.c.RenderToMainViews(types.RefreshMainOpts{ Pair: self.c.MainViewPairs().Normal, Main: &types.ViewUpdateOpts{ diff --git a/pkg/integration/tests/patch_building/keep_selection_after_moving_patch_out_main_view.go b/pkg/integration/tests/patch_building/keep_selection_after_moving_patch_out_main_view.go new file mode 100644 index 000000000..164fc72e5 --- /dev/null +++ b/pkg/integration/tests/patch_building/keep_selection_after_moving_patch_out_main_view.go @@ -0,0 +1,69 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepSelectionAfterMovingPatchOutMainView = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Moving a custom patch out of a commit from the focused main view re-establishes the (stale, multi-line) selection on a surviving change rather than leaving it painted over the shrunk diff", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\nfour\nfive\n") + shell.Commit("commit to move a patch out of") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move a patch out of").IsSelected(), + Contains("first commit"), + ). + // Focus the commit's diff straight from the commits panel, rather than + // entering the commit files panel first. + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("+one"), + ). + // Toggle just the first line into a custom patch, then leave a multi-line + // range selected — the patch move below doesn't go through the focused-main- + // view action handlers, so without the preserve net this stale range would be + // left painted over the shrunk diff. + PressPrimaryAction(). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+four")). + SelectedLines( + Contains("+one"), + Contains("+two"), + Contains("+three"), + Contains("+four"), + ) + + t.Common().SelectPatchOption(Contains("Move patch out into index")) + + // The moved line ('one') is gone from the commit, and the stale multi-line range + // collapses to a single surviving change at the selection's top ordinal (now the + // 'two' line) — just like discarding from the commit does. + t.Views().Main(). + IsFocused(). + Content(DoesNotContain("+one")). + ContainsLines( + Equals("+two"), + Equals("+three"), + Equals("+four"), + Equals("+five"), + ). + SelectedLines( + Contains("+two"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index a35b221f2..23bfff2de 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -369,6 +369,7 @@ var tests = []*components.IntegrationTest{ patch_building.DiscardLinesFromCommit, patch_building.DiscardLinesFromCommitMainView, patch_building.EditLineInPatchBuildingPanel, + patch_building.KeepSelectionAfterMovingPatchOutMainView, patch_building.MoveRangeToIndex, patch_building.MoveToEarlierCommit, patch_building.MoveToEarlierCommitFromAddedFile, @@ -540,6 +541,7 @@ var tests = []*components.IntegrationTest{ undo.UndoCheckoutAndDrop, undo.UndoCommit, undo.UndoDrop, + undo.UndoKeepsFocusedMainViewSelection, worktree.AddForExistingBranch, worktree.AddFromBranch, worktree.AddFromBranchDetached, diff --git a/pkg/integration/tests/undo/undo_keeps_focused_main_view_selection.go b/pkg/integration/tests/undo/undo_keeps_focused_main_view_selection.go new file mode 100644 index 000000000..e3e85629e --- /dev/null +++ b/pkg/integration/tests/undo/undo_keeps_focused_main_view_selection.go @@ -0,0 +1,86 @@ +package undo + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var UndoKeepsFocusedMainViewSelection = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Undoing a commit rewrite while focused in the main view re-establishes the (stale, multi-line) selection on a surviving change rather than leaving it painted over the changed diff", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\nfour\nfive\n") + shell.Commit("commit to rewrite") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to rewrite").IsSelected(), + Contains("first commit"), + ). + // Focus the commit's diff straight from the commits panel. + Press(keys.Universal.FocusMainView) + + // Discard the first line from the commit; the selection advances to the next + // surviving change ('two'). + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("+one"), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard lines from commit")). + Content(Equals("Are you sure you want to discard the selected lines from this commit?")). + Confirm() + + t.Views().Main(). + IsFocused(). + Content(DoesNotContain("+one")). + SelectedLines( + Contains("+two"), + ). + // Leave a multi-line range selected before undoing — undo rewrites the commit + // outside the focused-main-view action handlers, so without the preserve net + // this range would be left stale over the restored diff. + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+four")). + SelectedLines( + Contains("+two"), + Contains("+three"), + Contains("+four"), + ) + + // Undo is a global keybinding, so it fires while the main view holds focus. + t.GlobalPress(keys.Universal.Undo) + + t.ExpectPopup().Confirmation(). + Title(Equals("Undo")). + Content(MatchesRegexp(`Are you sure you want to hard reset to '.*'\?`)). + Confirm() + + // The discarded line is back, and the stale multi-line range collapses to a + // single surviving change at the selection's top ordinal (the first change line, + // now 'one' again) rather than spanning arbitrary lines of the restored diff. + t.Views().Main(). + IsFocused(). + ContainsLines( + Equals("+one"), + Equals("+two"), + Equals("+three"), + Equals("+four"), + Equals("+five"), + ). + SelectedLines( + Contains("+one"), + ) + }, +})