From 7bf398db1d0a13c173d30cb22c9c8e323d95bd52 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:09:48 +0100 Subject: [PATCH 1/8] Add tests demonstrating bugs when moving a patch to an earlier commit When moving a custom patch to an earlier commit, or into a new commit before the original one, lazygit applies the patch to the earlier commit and then replays the original commit unchanged. It relies on git noticing during the replay that the patch's changes are already there. This doesn't always work: - If the patch contains only some of several adjacent added lines, replaying the original commit stops with a conflict. - If git can place the patch's added lines in two positions because they repeat the line after them, the replay may put them in the other position than the patch did. Git then keeps both copies, and the file ends up containing the patch's lines twice. There's no conflict, so this goes unnoticed. When moving into a new commit before the original one, the conflict of the first case doesn't show the usual "Conflicts!" popup; lazygit focuses the commits panel instead. Co-Authored-By: Claude Opus 5.5 --- ...ly_placed_added_lines_to_earlier_commit.go | 72 ++++++++++++++++++ ...placed_added_lines_to_new_commit_before.go | 71 ++++++++++++++++++ ..._adjacent_added_lines_to_earlier_commit.go | 75 +++++++++++++++++++ ...jacent_added_lines_to_new_commit_before.go | 74 ++++++++++++++++++ ...patch_from_added_file_to_earlier_commit.go | 10 +++ pkg/integration/tests/test_list.go | 4 + 6 files changed, 306 insertions(+) create mode 100644 pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go create mode 100644 pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go create mode 100644 pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go create mode 100644 pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..7c2ec2c07 --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go @@ -0,0 +1,72 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to an earlier commit, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + /* EXPECTED: + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ACTUAL: */ + Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..e10cf853d --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go @@ -0,0 +1,71 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to a new commit before it, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + /* EXPECTED: + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ACTUAL: */ + Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..db30538cc --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go @@ -0,0 +1,75 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to an earlier commit", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + /* EXPECTED: + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }). + SelectPreviousItem(). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }) + ACTUAL: */ + t.Common().AcknowledgeConflicts() + + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU").Contains("file1"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..56c3ad038 --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go @@ -0,0 +1,74 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to a new commit before it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + /* EXPECTED: + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }). + SelectNextItem(). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }) + ACTUAL: */ + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("<-- CONFLICT --- commit to move from"), + Contains("─── Commits"), + Contains("new commit"), + Contains("first commit"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go index 1cad9e646..a726af2c5 100644 --- a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go @@ -44,6 +44,16 @@ var MovePatchFromAddedFileToEarlierCommit = NewIntegrationTest(NewIntegrationTes t.Views().Commits().Focus().SelectNextItem() t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + /* EXPECTED: + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ). + PressEnter() + ACTUAL: */ // The source commit tries to add a file that the moved line now causes to exist. t.Common().AcknowledgeConflicts() diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index acbe5b445..6e902bacb 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -449,9 +449,13 @@ var tests = []*components.IntegrationTest{ main_view.KeepSelectedRangeWhenChangingContextSize, main_view.KeepSelectionAfterMovingPatchOut, main_view.KeepSelectionVisibleWhenDiffShrinks, + main_view.MoveAmbiguouslyPlacedAddedLinesToEarlierCommit, + main_view.MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore, main_view.MoveMultiFileRangeToIndex, main_view.MoveOnWhenThePatchMarksRewrapTheDiff, + main_view.MovePartOfAdjacentAddedLinesToEarlierCommit, main_view.MovePartOfAdjacentAddedLinesToIndex, + main_view.MovePartOfAdjacentAddedLinesToNewCommitBefore, main_view.MovePartialPatchToIndex, main_view.MovePartialPatchToLaterCommit, main_view.MovePartialPatchToNewCommit, From 9383172a6c2ae4d8e5a9a41286a5d726cea61d65 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:44:43 +0100 Subject: [PATCH 2/8] Add a test demonstrating that the conflicts popup disappears If moving a custom patch into a new commit before the original one results in a conflict, lazygit shows the "Conflicts!" popup, but then immediately focuses the commits view. This closes the popup, and the user is left in a rebase that is stopped at a conflict without being told about it. The test uses a pre-commit hook that changes a file of the new commit; this makes picking the original commit conflict. Co-Authored-By: Claude Opus 5.5 --- ...atch_to_new_commit_before_with_conflict.go | 69 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 70 insertions(+) create mode 100644 pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go diff --git a/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go new file mode 100644 index 000000000..512cb1436 --- /dev/null +++ b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go @@ -0,0 +1,69 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePatchToNewCommitBeforeWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move a patch into a new commit before the original commit, where a commit hook changes the new commit so that replaying the original commit conflicts", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "1st line\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.CreateFileAndAdd("file2", "file2 content\n") + shell.Commit("commit to move from") + + shell.CreateFile(".git/hooks/pre-commit", "#!/bin/sh\necho 'hook line' >> file1\ngit add file1\n") + shell.MakeExecutable(".git/hooks/pre-commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("▼ /").IsSelected(), + Contains("file1"), + Contains("file2"), + ). + NavigateToLine(Contains("file2")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + /* EXPECTED: + t.Common().AcknowledgeConflicts() + + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU").Contains("file1"), + ) + ACTUAL: */ + t.Views().Commits(). + IsFocused(). + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("<-- CONFLICT --- commit to move from"), + Contains("─── Commits"), + Contains("new commit"), + Contains("first commit"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6e902bacb..13282ef5d 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -471,6 +471,7 @@ var tests = []*components.IntegrationTest{ main_view.MovePatchToLaterCommit, main_view.MovePatchToNewCommit, main_view.MovePatchToNewCommitBefore, + main_view.MovePatchToNewCommitBeforeWithConflict, main_view.MovePatchToNewCommitInStackedBranch, main_view.NavigateByHunkAndFile, main_view.NoDiffStatLinksUnderAnExternalDiff, From e8261e06d149d1f21f0e6507e3b82c767776a4eb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:46:29 +0100 Subject: [PATCH 3/8] Disable moving a patch between a merge commit and another commit If the patch comes from a merge commit, moving it to a later commit leaves the user in a rebase that is stuck before its first todo. Moving it to an earlier commit, or into a new commit before the merge, finishes, but with surprising results. The rebase doesn't replay a merge commit; it recreates it from its parents. If the patch contains changes from the merged branch, the commits on that branch that made them become empty. And a new commit "before" the merge ends up at the tip of the merged branch. Moving a patch to a later merge commit gets stuck in the same way. Moving a patch to a merge commit isn't useful in either direction, so disable both. The other patch commands amend the merge commit itself and work fine, so they stay enabled. They are useful for removing changes that were added to a merge commit by accident. Co-Authored-By: Claude Opus 5.5 --- .../custom_patch_options_menu_action.go | 24 +++++-- pkg/i18n/english.go | 4 ++ .../disable_moving_patch_from_merge_commit.go | 60 ++++++++++++++++ .../disable_moving_patch_to_merge_commit.go | 71 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 5 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go create mode 100644 pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index dc95c00df..f3ae211a5 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -48,6 +48,11 @@ func (self *CustomPatchOptionsMenuAction) Call() error { } if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState().None() { + var disabledReasonForMergeCommit *types.DisabledReason + if self.isPatchFromMergeCommit() { + disabledReasonForMergeCommit = &types.DisabledReason{Text: self.c.Tr.NotSupportedForPatchFromMergeCommit} + } + menuItems = append(menuItems, []*types.MenuItem{ { Label: fmt.Sprintf(self.c.Tr.RemovePatchFromOriginalCommit, utils.ShortHash(self.c.Git().Patch.PatchBuilder.To)), @@ -68,10 +73,11 @@ func (self *CustomPatchOptionsMenuAction) Call() error { Keys: menuKey('n'), }, { - Label: self.c.Tr.MovePatchIntoNewCommitBefore, - Tooltip: self.c.Tr.MovePatchIntoNewCommitBeforeTooltip, - OnPress: self.handlePullPatchIntoNewCommitBefore, - Keys: menuKey('N'), + Label: self.c.Tr.MovePatchIntoNewCommitBefore, + Tooltip: self.c.Tr.MovePatchIntoNewCommitBeforeTooltip, + OnPress: self.handlePullPatchIntoNewCommitBefore, + Keys: menuKey('N'), + DisabledReason: disabledReasonForMergeCommit, }, }...) @@ -79,7 +85,10 @@ func (self *CustomPatchOptionsMenuAction) Call() error { selectedCommit := self.c.Contexts().LocalCommits.GetSelected() if selectedCommit != nil && self.c.Git().Patch.PatchBuilder.To != selectedCommit.Hash() { - var disabledReason *types.DisabledReason + disabledReason := disabledReasonForMergeCommit + if selectedCommit.IsMerge() { + disabledReason = &types.DisabledReason{Text: self.c.Tr.CannotMovePatchToMergeCommit} + } if self.c.Contexts().LocalCommits.AreMultipleItemsSelected() { disabledReason = &types.DisabledReason{Text: self.c.Tr.RangeSelectNotSupported} } @@ -123,6 +132,11 @@ func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { return -1 } +func (self *CustomPatchOptionsMenuAction) isPatchFromMergeCommit() bool { + commitIndex := self.getPatchCommitIndex() + return commitIndex != -1 && self.c.Model().Commits[commitIndex].IsMerge() +} + func (self *CustomPatchOptionsMenuAction) handleDeletePatchFromCommit() error { commits := self.c.Model().Commits commitIndex := self.getPatchCommitIndex() diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0c8ed7d74..05a74b0fe 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -1013,6 +1013,8 @@ type TranslationSet struct { RangeSelectUp string RangeSelectDown string RangeSelectNotSupported string + NotSupportedForPatchFromMergeCommit string + CannotMovePatchToMergeCommit string NoItemSelected string SelectedItemIsNotABranch string SelectedItemDoesNotHaveFiles string @@ -2230,6 +2232,8 @@ func EnglishTranslationSet() *TranslationSet { RangeSelectUp: "Range select up", RangeSelectDown: "Range select down", RangeSelectNotSupported: "Action does not support range selection, please select a single item", + NotSupportedForPatchFromMergeCommit: "Not supported for a patch from a merge commit", + CannotMovePatchToMergeCommit: "Cannot move a patch to a merge commit", NoItemSelected: "No item selected", SelectedItemIsNotABranch: "Selected item is not a branch", SelectedItemDoesNotHaveFiles: "Selected item does not have files to view", diff --git a/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go b/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go new file mode 100644 index 000000000..59cf687a9 --- /dev/null +++ b/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go @@ -0,0 +1,60 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DisableMovingPatchFromMergeCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Moving a patch from a merge commit to another commit, or into a new commit before it, is disabled", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + NewBranch("main-branch"). + CreateFileAndAdd("file1", "file1 content\n"). + Commit("first commit"). + NewBranch("feature"). + CreateFileAndAdd("file2", "file2 content\n"). + Commit("feature commit"). + Checkout("main-branch"). + CreateFileAndAdd("file3", "file3 content\n"). + Commit("main commit"). + Merge("feature") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("Merge branch 'feature' into main-branch").IsSelected(), + Contains("feature commit"), + Contains("main commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file2").IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Commits(). + Focus(). + NavigateToLine(Contains("first commit")) + + t.GlobalPress(keys.Universal.CreatePatchOptionsMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Patch options")). + Select(Contains("Move patch to selected commit")). + Tooltip(Contains("Disabled: Not supported for a patch from a merge commit")). + Select(Contains("Move patch into new commit before the original commit")). + Tooltip(Contains("Disabled: Not supported for a patch from a merge commit")). + Cancel() + }, +}) diff --git a/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go b/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go new file mode 100644 index 000000000..aebeec9ac --- /dev/null +++ b/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go @@ -0,0 +1,71 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DisableMovingPatchToMergeCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Moving a patch to an earlier or later merge commit is disabled", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + NewBranch("main-branch"). + CreateFileAndAdd("file1", "file1 content\n"). + Commit("first commit"). + NewBranch("feature"). + CreateFileAndAdd("file2", "file2 content\n"). + Commit("feature commit"). + Checkout("main-branch"). + CreateFileAndAdd("file3", "file3 content\n"). + Commit("main commit"). + Merge("feature"). + CreateFileAndAdd("file4", "file4 content\n"). + Commit("top commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + checkMovingPatchToMergeCommitIsDisabled := func(commitToMoveFrom string, file string) { + t.Views().Commits(). + Focus(). + NavigateToLine(Contains(commitToMoveFrom)). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains(file).IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Commits(). + Focus(). + NavigateToLine(Contains("Merge branch 'feature' into main-branch")) + + t.GlobalPress(keys.Universal.CreatePatchOptionsMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Patch options")). + Select(Contains("Move patch to selected commit")). + Tooltip(Contains("Disabled: Cannot move a patch to a merge commit")). + Cancel() + + t.Common().ResetCustomPatch() + } + + t.Views().Commits(). + Lines( + Contains("top commit").IsSelected(), + Contains("Merge branch 'feature' into main-branch"), + Contains("feature commit"), + Contains("main commit"), + Contains("first commit"), + ) + + checkMovingPatchToMergeCommitIsDisabled("top commit", "file4") + checkMovingPatchToMergeCommitIsDisabled("main commit", "file3") + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 13282ef5d..559816011 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -407,6 +407,8 @@ var tests = []*components.IntegrationTest{ main_view.CopySelectedDiffLines, main_view.CustomPatchGoesThroughTheDiffRenderer, main_view.CustomPatchIgnoresLineEndingConversion, + main_view.DisableMovingPatchFromMergeCommit, + main_view.DisableMovingPatchToMergeCommit, main_view.DiscardAllChanges, main_view.DiscardDiffLines, main_view.DiscardFromACommitOnlyWhereItCanBeRewritten, From 09068b03604de46187606b26604ba0717305a053 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:13:18 +0100 Subject: [PATCH 4/8] Add a function to change which commit a rebase todo picks The next commit uses this to replay a modified copy of a commit in place of the original one. Co-Authored-By: Claude Opus 5.5 --- pkg/utils/rebase_todo.go | 27 ++++++++++++++ pkg/utils/rebase_todo_test.go | 69 +++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 3a3577f80..577b3da07 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -331,3 +331,30 @@ func dropMergeCommit(todos []todo.Todo, hash string) ([]todo.Todo, error) { _, idx, _ := lo.FindIndexOf(todos, isMerge) return slices.Delete(todos, idx, idx+1), nil } + +// Read a git-rebase-todo file, make the todo for the given commit use a +// different commit instead, and write it back +func ChangeTodoCommit(fileName string, hash string, newHash string, commentChar byte) error { + todos, err := ReadRebaseTodoFile(fileName, commentChar) + if err != nil { + return err + } + + newTodos, err := changeTodoCommit(todos, hash, newHash) + if err != nil { + return err + } + + return WriteRebaseTodoFile(fileName, newTodos, commentChar) +} + +func changeTodoCommit(todos []todo.Todo, hash string, newHash string) ([]todo.Todo, error) { + idx, ok := findTodo(todos, Todo{Hash: hash}) + if !ok { + // Should never happen + return nil, fmt.Errorf("Todo %s not found in git-rebase-todo", hash) + } + + todos[idx].Commit = newHash + return todos, nil +} diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index a9ac1bba5..b032e4057 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -596,6 +596,75 @@ func TestRebaseCommands_deleteTodos(t *testing.T) { } } +func TestRebaseCommands_changeTodoCommit(t *testing.T) { + scenarios := []struct { + name string + todos []todo.Todo + hash string + newHash string + expectedTodos []todo.Todo + expectedErr error + }{ + { + name: "success", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234", Msg: "first"}, + {Command: todo.Pick, Commit: "5678", Msg: "second"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "abcd", Msg: "third"}, + }, + hash: "5678", + newHash: "ef01", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234", Msg: "first"}, + {Command: todo.Pick, Commit: "ef01", Msg: "second"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "abcd", Msg: "third"}, + }, + expectedErr: nil, + }, + { + name: "abbreviated hash in todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + hash: "56789abc", + newHash: "ef01", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "ef01"}, + }, + expectedErr: nil, + }, + { + name: "failure", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + hash: "abcd", + newHash: "ef01", + expectedTodos: nil, + expectedErr: errors.New("Todo abcd not found in git-rebase-todo"), + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + actualTodos, actualErr := changeTodoCommit(scenario.todos, scenario.hash, scenario.newHash) + + if scenario.expectedErr == nil { + assert.NoError(t, actualErr) + } else { + assert.EqualError(t, actualErr, scenario.expectedErr.Error()) + } + + assert.EqualValues(t, scenario.expectedTodos, actualTodos) + }) + } +} + func Test_equalHash(t *testing.T) { scenarios := []struct { a string From 507b65065f6c73fc3472f860d43dd0f50d44d034 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:19:25 +0100 Subject: [PATCH 5/8] Avoid conflicts and duplicate lines when moving a patch to an earlier commit When moving a custom patch to an earlier commit, we apply the patch to the earlier commit and then let the rebase pick the commit that the patch came from, unchanged. This relies on git noticing during the pick that the patch's changes are already there. If the patch contains only some of several adjacent added lines, git doesn't notice, and the pick stops with a conflict. If git can place the patch's added lines in more than one position, it may place them differently for the pick than for the patch. It then adds them a second time, without any conflict. Make the rebase pick a copy of the source commit instead. The copy has the same tree, author and message, but its parent is the original parent with the patch applied. Picking it only replays the source commit's changes minus the patch, so git no longer has to recognize the patch as already applied. Create the copy with plumbing commands and a temporary index; this doesn't touch the working tree. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/patch.go | 108 ++++++++++++++++++ pkg/commands/git_commands/patch_test.go | 76 ++++++++++++ pkg/commands/git_commands/rebase.go | 11 ++ ...ly_placed_added_lines_to_earlier_commit.go | 3 - ..._adjacent_added_lines_to_earlier_commit.go | 9 -- ...patch_from_added_file_to_earlier_commit.go | 37 ------ 6 files changed, 195 insertions(+), 49 deletions(-) create mode 100644 pkg/commands/git_commands/patch_test.go diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 39f60478a..f3d6d9647 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -211,6 +211,11 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s return err } + if err := self.pickCopyWithPatchInParent(commits[sourceCommitIdx]); err != nil { + _ = self.rebase.AbortRebase() + return err + } + // apply each patch forward if err := self.ApplyCustomPatch(false, false); err != nil { // Don't abort the rebase here; this might cause conflicts, so give @@ -427,6 +432,109 @@ func (self *PatchCommands) PullPatchIntoNewCommitBefore( return self.rebase.ContinueRebase() } +// When moving the patch to an earlier commit, we apply it to that commit, and +// the rebase then picks the commit that the patch was built from. Picking that +// commit unchanged would rely on git noticing that the patch is already +// applied, and git doesn't always notice this. If the patch contains only some +// of several adjacent added lines, the pick conflicts; if git can place the +// patch's added lines in more than one position, it may add them a second +// time. So we make the rebase pick a copy of the commit instead, whose parent +// already contains the patch. +func (self *PatchCommands) pickCopyWithPatchInParent(commit *models.Commit) error { + copyHash, err := self.copyCommitWithPatchInParent(commit) + if err != nil { + return err + } + + return self.rebase.ChangeTodoCommit(commit.Hash(), copyHash) +} + +// Creates a copy of the given commit, with the same tree, author, and message, +// whose parent is the commit's original parent with the custom patch applied. +// It does this without touching the working tree or the index. +func (self *PatchCommands) copyCommitWithPatchInParent(commit *models.Commit) (string, error) { + indexDir, err := os.MkdirTemp(self.os.GetTempDir(), "index") + if err != nil { + return "", err + } + defer os.RemoveAll(indexDir) + indexFileEnvVar := "GIT_INDEX_FILE=" + filepath.Join(indexDir, "index") + + cmd := self.cmd.New(NewGitCmd("read-tree").Arg(commit.Parents()[0]).ToArgv()) + err = cmd.AddEnvVars(indexFileEnvVar).DontLog().Run() + if err != nil { + return "", err + } + + patchFilePath, err := self.SaveTemporaryPatch(self.PatchBuilder.PatchToApply(false, false)) + if err != nil { + return "", err + } + cmd = self.cmd.New(NewGitCmd("apply").Arg("--cached", patchFilePath).ToArgv()) + err = cmd.AddEnvVars(indexFileEnvVar).DontLog().Run() + if err != nil { + return "", err + } + + cmd = self.cmd.New(NewGitCmd("write-tree").ToArgv()) + tree, _, err := cmd.AddEnvVars(indexFileEnvVar).DontLog().RunWithOutputs() + if err != nil { + return "", err + } + + cmd = self.cmd.New(NewGitCmd("cat-file").Arg("commit", commit.Hash()).ToArgv()) + commitObject, _, err := cmd.DontLog().RunWithOutputs() + if err != nil { + return "", err + } + + parentHash, err := self.writeCommitObject( + commitObjectWithHeader(commitObject, "tree", strings.TrimSpace(tree))) + if err != nil { + return "", err + } + + return self.writeCommitObject(commitObjectWithHeader(commitObject, "parent", parentHash)) +} + +func (self *PatchCommands) writeCommitObject(commitObject string) (string, error) { + cmd := self.cmd.New( + NewGitCmd("hash-object"). + Arg("-t", "commit", "-w", "--stdin").ToArgv()) + hash, _, err := cmd. + SetStdin(commitObject). + DontLog(). + RunWithOutputs() + return strings.TrimSpace(hash), err +} + +// Returns the given raw commit object with the value of the given header +// replaced. The commit's signature is dropped, since it no longer matches. +func commitObjectWithHeader(commitObject string, name string, value string) string { + headers, message, _ := strings.Cut(commitObject, "\n\n") + + newHeaders := []string{} + isInSignature := false + for line := range strings.SplitSeq(headers, "\n") { + // Continuation lines of a multi-line header start with a space + if isInSignature && strings.HasPrefix(line, " ") { + continue + } + + isInSignature = strings.HasPrefix(line, "gpgsig ") || strings.HasPrefix(line, "gpgsig-sha256 ") + if isInSignature { + continue + } + + if strings.HasPrefix(line, name+" ") { + line = name + " " + value + } + newHeaders = append(newHeaders, line) + } + + return strings.Join(newHeaders, "\n") + "\n\n" + message +} + // We have just applied a patch in reverse to discard it from a commit; if we // now try to apply the patch again to move it to a later commit, or to the // index, then this would conflict "with itself" in case the patch contained diff --git a/pkg/commands/git_commands/patch_test.go b/pkg/commands/git_commands/patch_test.go new file mode 100644 index 000000000..ab7518df9 --- /dev/null +++ b/pkg/commands/git_commands/patch_test.go @@ -0,0 +1,76 @@ +package git_commands + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCommitObjectWithHeader(t *testing.T) { + scenarios := []struct { + name string + commitObject string + headerName string + value string + expected string + }{ + { + name: "replace parent", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "\nsubject\n\nbody\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "\nsubject\n\nbody\n", + }, + { + name: "replace tree, keeping other headers", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "encoding ISO-8859-1\n" + + "\nsubject\n", + headerName: "tree", + value: "3333", + expected: "tree 3333\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "encoding ISO-8859-1\n" + + "\nsubject\n", + }, + { + name: "drop signature", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "gpgsig -----BEGIN PGP SIGNATURE-----\n \n iQEz\n -----END PGP SIGNATURE-----\n" + + "gpgsig-sha256 -----BEGIN SSH SIGNATURE-----\n U1NI\n -----END SSH SIGNATURE-----\n" + + "some-other-header value\n" + + "\nsubject\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "some-other-header value\n" + + "\nsubject\n", + }, + { + name: "empty message", + commitObject: "tree 1111\nparent 2222\n\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n\n", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, commitObjectWithHeader(s.commitObject, s.headerName, s.value)) + }) + } +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 74278b18d..c79105c1f 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -352,6 +352,17 @@ func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo ) } +// Makes the todo for the given commit in the git-rebase-todo file use a +// different commit instead +func (self *RebaseCommands) ChangeTodoCommit(hash string, newHash string) error { + return utils.ChangeTodoCommit( + filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"), + hash, + newHash, + self.config.GetCoreCommentChar(), + ) +} + func (self *RebaseCommands) DeleteUpdateRefTodos(commits []*models.Commit) error { todosToDelete := lo.Map(commits, func(commit *models.Commit, _ int) utils.Todo { return todoFromCommit(commit) diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go index 7c2ec2c07..3a18daff6 100644 --- a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go @@ -63,10 +63,7 @@ var MoveAmbiguouslyPlacedAddedLinesToEarlierCommit = NewIntegrationTest(NewInteg t.FileSystem().FileContent( "file1", - /* EXPECTED: Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), - ACTUAL: */ - Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), ) }, }) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go index db30538cc..ee959326c 100644 --- a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go @@ -48,7 +48,6 @@ var MovePartOfAdjacentAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrat t.Views().Commits().Focus().SelectNextItem() t.Common().SelectPatchOption(Contains("Move patch to selected commit")) - /* EXPECTED: t.Views().Commits(). IsFocused(). Lines( @@ -63,13 +62,5 @@ var MovePartOfAdjacentAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrat Tap(func() { t.Views().Main().Content(Contains(" 1st line\n+2nd line")) }) - ACTUAL: */ - t.Common().AcknowledgeConflicts() - - t.Views().Files(). - IsFocused(). - Lines( - Contains("UU").Contains("file1"), - ) }, }) diff --git a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go index a726af2c5..2c583af90 100644 --- a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go @@ -44,7 +44,6 @@ var MovePatchFromAddedFileToEarlierCommit = NewIntegrationTest(NewIntegrationTes t.Views().Commits().Focus().SelectNextItem() t.Common().SelectPatchOption(Contains("Move patch to selected commit")) - /* EXPECTED: t.Views().Commits(). IsFocused(). Lines( @@ -53,42 +52,6 @@ var MovePatchFromAddedFileToEarlierCommit = NewIntegrationTest(NewIntegrationTes Contains("first commit"), ). PressEnter() - ACTUAL: */ - // The source commit tries to add a file that the moved line now causes to exist. - t.Common().AcknowledgeConflicts() - - t.Views().Files(). - IsFocused(). - Lines( - Contains("AA").Contains("file"), - ). - PressEnter() - - t.Views().MergeConflicts(). - IsFocused(). - TopLines( - Contains("<<<<<<< HEAD"), - Contains("2nd line"), - Contains("======="), - Contains("1st line"), - Contains("2nd line"), - Contains("3rd line"), - Contains(">>>>>>>"), - ). - SelectNextItem(). - PressPrimaryAction() - - t.Common().ContinueOnConflictsResolved("rebase") - - t.Views().Commits(). - Focus(). - Lines( - Contains("commit to move from").IsSelected(), - Contains("destination commit"), - Contains("first commit"), - ). - NavigateToLine(Contains("destination commit")). - PressEnter() t.Views().CommitFiles(). IsFocused(). From 16f51df6e3abe773cc23dfdd91dd1986b82398b1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:21:01 +0100 Subject: [PATCH 6/8] Avoid conflicts and duplicate lines when moving a patch into a new commit before Moving a custom patch into a new commit before the original one has the same problem as moving it to an earlier commit. After committing the patch, the rebase picks the original commit unchanged and relies on git noticing that the patch is already there. Make the rebase pick a copy of the original commit whose parent already contains the patch, as we do when moving to an earlier commit. Here, the copy's parent normally has the same tree as the new commit, so picking the copy doesn't conflict. Only a commit hook that changes the new commit's files could make it conflict. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/patch.go | 22 ++++++++++++------- ...placed_added_lines_to_new_commit_before.go | 3 --- ...jacent_added_lines_to_new_commit_before.go | 9 -------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index f3d6d9647..28e6c54ad 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -419,6 +419,11 @@ func (self *PatchCommands) PullPatchIntoNewCommitBefore( return err } + if err := self.pickCopyWithPatchInParent(commits[commitIdx]); err != nil { + _ = self.rebase.AbortRebase() + return err + } + if err := self.ApplyCustomPatch(false, false); err != nil { _ = self.rebase.AbortRebase() return err @@ -432,14 +437,15 @@ func (self *PatchCommands) PullPatchIntoNewCommitBefore( return self.rebase.ContinueRebase() } -// When moving the patch to an earlier commit, we apply it to that commit, and -// the rebase then picks the commit that the patch was built from. Picking that -// commit unchanged would rely on git noticing that the patch is already -// applied, and git doesn't always notice this. If the patch contains only some -// of several adjacent added lines, the pick conflicts; if git can place the -// patch's added lines in more than one position, it may add them a second -// time. So we make the rebase pick a copy of the commit instead, whose parent -// already contains the patch. +// When moving the patch to an earlier commit, or into a new commit before the +// one it was built from, we first apply it there, and the rebase then picks +// the commit that the patch was built from. Picking that commit unchanged +// would rely on git noticing that the patch is already applied, and git +// doesn't always notice this. If the patch contains only some of several +// adjacent added lines, the pick conflicts; if git can place the patch's added +// lines in more than one position, it may add them a second time. So we make +// the rebase pick a copy of the commit instead, whose parent already contains +// the patch. func (self *PatchCommands) pickCopyWithPatchInParent(commit *models.Commit) error { copyHash, err := self.copyCommitWithPatchInParent(commit) if err != nil { diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go index e10cf853d..275411f1d 100644 --- a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go @@ -62,10 +62,7 @@ var MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore = NewIntegrationTest(NewInt t.FileSystem().FileContent( "file1", - /* EXPECTED: Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), - ACTUAL: */ - Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), ) }, }) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go index 56c3ad038..5642ae6a5 100644 --- a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go @@ -49,7 +49,6 @@ var MovePartOfAdjacentAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegr t.Views().Commits(). IsFocused(). - /* EXPECTED: Lines( Contains("commit to move from").IsSelected(), Contains("new commit"), @@ -62,13 +61,5 @@ var MovePartOfAdjacentAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegr Tap(func() { t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) }) - ACTUAL: */ - Lines( - Contains("─── Pending rebase todos"), - Contains("pick").Contains("<-- CONFLICT --- commit to move from"), - Contains("─── Commits"), - Contains("new commit"), - Contains("first commit"), - ) }, }) From 3864ddc52c5e28b4b4acc65831eca8e507d89672 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:48:42 +0100 Subject: [PATCH 7/8] Unify the handlers for moving a patch into a new commit The handlers for moving the patch into a new commit before or after the original one differ only in the git command they call. The next commit fixes a bug that both have. Co-Authored-By: Claude Opus 5.5 --- .../custom_patch_options_menu_action.go | 42 +++++-------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index f3ae211a5..164c54af4 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -176,39 +176,14 @@ func (self *CustomPatchOptionsMenuAction) handleMovePatchIntoWorkingTree() error } func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error { - commitIndex := self.getPatchCommitIndex() - self.c.Helpers().Commits.OpenCommitMessagePanel( - &helpers.OpenCommitMessagePanelOpts{ - // Pass a commit index of one less than the moved-from commit, so that - // you can press up arrow once to recall the original commit message: - CommitIndex: commitIndex - 1, - InitialMessage: "", - SummaryTitle: self.c.Tr.CommitSummaryTitle, - DescriptionTitle: self.c.Tr.CommitDescriptionTitle, - PreserveMessage: false, - OnConfirm: func(summary string, description string) error { - commits := self.c.Model().Commits - self.c.Helpers().Commits.CloseCommitMessagePanel() - return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { - return err - } - self.c.OnUIThread(func() error { - self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) - return nil - }) - return nil - }) - }, - }, - ) - - return nil + return self.pullPatchIntoNewCommit(false) } func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() error { + return self.pullPatchIntoNewCommit(true) +} + +func (self *CustomPatchOptionsMenuAction) pullPatchIntoNewCommit(beforeOriginalCommit bool) error { commitIndex := self.getPatchCommitIndex() self.c.Helpers().Commits.OpenCommitMessagePanel( &helpers.OpenCommitMessagePanelOpts{ @@ -224,7 +199,12 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e self.c.Helpers().Commits.CloseCommitMessagePanel() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) + var err error + if beforeOriginalCommit { + err = self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) + } else { + err = self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) + } if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { return err } From fabc14d7e5f606c3619aaae74b43be170527065c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:49:35 +0100 Subject: [PATCH 8/8] Keep the conflicts popup open after moving a patch into a new commit If moving a custom patch into a new commit stops at a conflict, CheckMergeOrRebase shows the "Conflicts!" popup. The handler then focused the commits view anyway, and this closed the popup. Only focus the commits view if the operation finished. Co-Authored-By: Claude Opus 5.5 --- .../custom_patch_options_menu_action.go | 14 ++++++++++---- ...ove_patch_to_new_commit_before_with_conflict.go | 11 ----------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index 164c54af4..bb7bfa7ef 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -199,15 +199,21 @@ func (self *CustomPatchOptionsMenuAction) pullPatchIntoNewCommit(beforeOriginalC self.c.Helpers().Commits.CloseCommitMessagePanel() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - var err error + var result error if beforeOriginalCommit { - err = self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) + result = self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) } else { - err = self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) + result = self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) } - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil { return err } + // If the rebase stopped at conflicts, CheckMergeOrRebase has asked the + // user how to handle them; focusing the commits view would close that + // popup. + if result != nil { + return nil + } self.c.OnUIThread(func() error { self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) return nil diff --git a/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go index 512cb1436..5d2c8fcfd 100644 --- a/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go +++ b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go @@ -47,7 +47,6 @@ var MovePatchToNewCommitBeforeWithConflict = NewIntegrationTest(NewIntegrationTe InitialText(Equals("")). Type("new commit").Confirm() - /* EXPECTED: t.Common().AcknowledgeConflicts() t.Views().Files(). @@ -55,15 +54,5 @@ var MovePatchToNewCommitBeforeWithConflict = NewIntegrationTest(NewIntegrationTe Lines( Contains("UU").Contains("file1"), ) - ACTUAL: */ - t.Views().Commits(). - IsFocused(). - Lines( - Contains("─── Pending rebase todos"), - Contains("pick").Contains("<-- CONFLICT --- commit to move from"), - Contains("─── Commits"), - Contains("new commit"), - Contains("first commit"), - ) }, })