diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index cde2a3424..28cf9b7c9 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -592,7 +592,7 @@ func (self *FilesController) applyDiffLineSelection(mainViewName string, firstLi // The Refresh above queued the main-view re-render; re-establish the selection at its // change-line ordinal once that render lands, and focus the pane now holding the // acted-on side. - revealSelectionAfterPrimaryAction(self.c, mainViewName, focusViewName, firstLineIdx) + revealSelectionAfterPrimaryAction(self.c, mainViewName, focusViewName, firstLineIdx, 0) if focusViewName != mainViewName { self.c.Context().Push(mainContextForViewName(self.c, focusViewName), types.OnFocusOpts{}) } diff --git a/pkg/gui/controllers/helpers/staging_helper.go b/pkg/gui/controllers/helpers/staging_helper.go index bd015e24f..2c6a0412b 100644 --- a/pkg/gui/controllers/helpers/staging_helper.go +++ b/pkg/gui/controllers/helpers/staging_helper.go @@ -354,12 +354,18 @@ func (self *StagingHelper) RestoreFocusedMainViewOnEscape(explorerView, mainView // the acted-on side to the other pane (e.g. unstaging the first hunk of an only-staged // file splits it, pushing the staged remainder to the secondary half); the ordinal is // preserved across that move since both panes show the same side. -func (self *StagingHelper) RevealSelectionAfterStaging(sourceView *gocui.View, targetView *gocui.View, firstLine int, place func(viewLine int)) { +// +// advanceBy shifts the landed ordinal forward by that many change lines. Staging and +// unstaging consume the acted-on lines, so the preserved ordinal (advanceBy 0) already +// lands on the next change. A custom-patch toggle leaves the diff unchanged, so preserving +// the ordinal would land back on the just-toggled lines; passing the toggled change-line +// count advances past them to the next stageable hunk/line, matching the staging feel. +func (self *StagingHelper) RevealSelectionAfterStaging(sourceView *gocui.View, targetView *gocui.View, firstLine int, advanceBy int, place func(viewLine int)) { ordinal, ok := self.changeLineOrdinal(sourceView, firstLine) if !ok { return } - self.revealChangeLineAtOrdinal(targetView, ordinal, place) + self.revealChangeLineAtOrdinal(targetView, ordinal+advanceBy, place) } // changeLineOrdinal returns how many change lines precede the (change) line at diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index aa3746b5b..d42ecf341 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -488,8 +488,11 @@ func (self *MainViewController) usingExternalDiff() bool { // sourceViewName and targetViewName are usually the same pane, but staging can move the // acted-on side to the other pane (passing that pane as the target). The target inherits // the (collapsed) select mode; a range collapses back to a single line, hunk mode stays -// on to land on the next hunk. -func revealSelectionAfterPrimaryAction(c *ControllerCommon, sourceViewName string, targetViewName string, firstLineIdx int) { +// on to land on the next hunk. advanceBy is forwarded to RevealSelectionAfterStaging: 0 for +// staging/removal (the acted-on lines are consumed), the toggled change-line count for a +// custom-patch toggle (which leaves the diff intact, so the selection must be advanced past +// the just-toggled lines to land on the next stageable hunk). +func revealSelectionAfterPrimaryAction(c *ControllerCommon, sourceViewName string, targetViewName string, firstLineIdx int, advanceBy int) { sourceContext := mainContextForViewName(c, sourceViewName) targetContext := mainContextForViewName(c, targetViewName) @@ -503,7 +506,7 @@ func revealSelectionAfterPrimaryAction(c *ControllerCommon, sourceViewName strin sourceView := sourceContext.GetView() targetView := targetContext.GetView() - c.Helpers().Staging.RevealSelectionAfterStaging(sourceView, targetView, firstLineIdx, func(viewLine int) { + c.Helpers().Staging.RevealSelectionAfterStaging(sourceView, targetView, firstLineIdx, advanceBy, func(viewLine int) { if mode == context.DiffSelectModeHunk { selectDiffHunk(c, targetContext, viewLine) } else { @@ -558,7 +561,7 @@ func preserveFocusedMainViewSelectionAcrossContentChange(c *ControllerCommon, ma // 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) + revealSelectionAfterPrimaryAction(c, mainContext.GetViewName(), mainContext.GetViewName(), first, 0) } // diffTaskCommandKey returns the buffer-manager task key a diff render task will register diff --git a/pkg/gui/controllers/patch_building_from_main_view.go b/pkg/gui/controllers/patch_building_from_main_view.go index f446cd78f..8e6bc6c96 100644 --- a/pkg/gui/controllers/patch_building_from_main_view.go +++ b/pkg/gui/controllers/patch_building_from_main_view.go @@ -76,9 +76,10 @@ func removePatchLinesFromFocusedMainView( } refresh() - // A removal doesn't change the diff command, so source and target are the same pane; - // the re-render still moves the selection in view-line space, so re-establish it. - revealSelectionAfterPrimaryAction(c, mainViewName, mainViewName, firstLineIdx) + // A removal shrinks the patch shown in the secondary, so the removed lines are consumed + // from its diff; preserving the ordinal (advanceBy 0) lands on the next surviving line, + // like unstaging. + revealSelectionAfterPrimaryAction(c, mainViewName, mainViewName, firstLineIdx, 0) return nil } @@ -132,10 +133,12 @@ func togglePatchFromFocusedMainView( } refresh() - // A toggle doesn't change the diff, so source and target are the same pane; - // the re-render (and the layout re-wrap when the secondary view first appears) - // still moves the selection in view-line space, so re-establish it. - revealSelectionAfterPrimaryAction(c, mainViewName, mainViewName, firstLineIdx) + // A toggle doesn't change the diff, so source and target are the same pane; the + // re-render (and the layout re-wrap when the secondary view first appears) still + // moves the selection in view-line space, so re-establish it — advanced past the + // just-toggled change lines to the next stageable hunk/line, since they aren't + // consumed (the diff is unchanged), unlike staging. + revealSelectionAfterPrimaryAction(c, mainViewName, mainViewName, firstLineIdx, len(infos)) return nil }, }) diff --git a/pkg/integration/tests/patch_building/build_from_main_view.go b/pkg/integration/tests/patch_building/build_from_main_view.go index bd80fc871..93b8b3ccf 100644 --- a/pkg/integration/tests/patch_building/build_from_main_view.go +++ b/pkg/integration/tests/patch_building/build_from_main_view.go @@ -62,12 +62,13 @@ var BuildFromMainView = NewIntegrationTest(NewIntegrationTestArgs{ Contains("+THREE"), ). PressPrimaryAction(). - // The selection is re-established on the same hunk after the toggle's + // After toggling the block in, the selection advances to the next stageable + // hunk (the NINE block) — like staging advances to the next hunk — across the // re-render (which, when the secondary view first appears, also re-wraps the // narrower diff). SelectedLines( - Contains("-three"), - Contains("+THREE"), + Contains("-nine"), + Contains("+NINE"), ) t.Views().Information().Content(Contains("Building patch")) diff --git a/pkg/integration/tests/patch_building/build_from_whole_commit_main_view.go b/pkg/integration/tests/patch_building/build_from_whole_commit_main_view.go index 57c1a200e..efd8b2592 100644 --- a/pkg/integration/tests/patch_building/build_from_whole_commit_main_view.go +++ b/pkg/integration/tests/patch_building/build_from_whole_commit_main_view.go @@ -61,12 +61,12 @@ var BuildFromWholeCommitMainView = NewIntegrationTest(NewIntegrationTestArgs{ Contains("+THREE"), ). PressPrimaryAction(). - // The selection is re-established on the same block after the toggle's - // re-render (which, when the secondary view first appears, re-wraps the - // narrower diff). + // After toggling the block in, the selection advances to the next stageable + // block — file2's BETA block — across the re-render (which, when the secondary + // view first appears, re-wraps the narrower diff). SelectedLines( - Contains("-three"), - Contains("+THREE"), + Contains("-beta"), + Contains("+BETA"), ) t.Views().Information().Content(Contains("Building patch")) diff --git a/pkg/integration/tests/patch_building/build_multi_file_from_whole_commit_main_view.go b/pkg/integration/tests/patch_building/build_multi_file_from_whole_commit_main_view.go index 0dd5d1f08..853707dd0 100644 --- a/pkg/integration/tests/patch_building/build_multi_file_from_whole_commit_main_view.go +++ b/pkg/integration/tests/patch_building/build_multi_file_from_whole_commit_main_view.go @@ -48,12 +48,8 @@ var BuildMultiFileFromWholeCommitMainView = NewIntegrationTest(NewIntegrationTes Contains("+THREE"), ). PressPrimaryAction(). - SelectedLines( - Contains("-three"), - Contains("+THREE"), - ). - // Move to the next change block, which is in file2, and toggle it in too. - Press(keys.Universal.NextItem). + // Toggling the block in advances the selection to the next stageable block, + // which crosses the file boundary into file2's BETA block; toggle it in too. SelectedLines( Contains("-beta"), Contains("+BETA"), 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 index 164fc72e5..f0d44a968 100644 --- 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 @@ -37,8 +37,10 @@ var KeepSelectionAfterMovingPatchOutMainView = NewIntegrationTest(NewIntegration // 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. + // left painted over the shrunk diff. (Toggling advances the selection to the + // next line, so go back to 'one' to anchor a range that spans the toggled line.) PressPrimaryAction(). + NavigateToLine(Contains("+one")). Press(keys.Universal.ToggleRangeSelect). NavigateToLine(Contains("+four")). SelectedLines(