From 16f51df6e3abe773cc23dfdd91dd1986b82398b1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:21:01 +0100 Subject: [PATCH] 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"), - ) }, })