From 53211012764e769fec812d86e790ba482e44098d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 9 May 2025 22:35:08 +0200 Subject: [PATCH] Use 'break' instead of 'edit' for BeginInteractiveRebaseForCommit with merge commit BeginInteractiveRebaseForCommit is used for all the patch commands, and for rewording. It works by setting the commit we want to stop at to 'edit'; this doesn't work for merge commits. This wasn't a problem for the patch commands so far, because you typically don't use custom patches with merge commits (although we don't prevent this; maybe we should?). However, it was a problem when you tried to reword a merge commit; this previously failed with an error, as the test added in the previous commit demonstrated. Also, we want to add a new patch command that has to stop *before* the selected commit (pull patch to new commit before the original one), and this wouldn't work for the first commit in a feature branch, because it would have to set the last commit before that to 'edit', which isn't possible if that's a merge (which is likely). To fix all this, use a 'break' before the selected commit if the commit is a merge. It is important that we only do it in that case and not always, otherwise we would break the new regression tests that were added a few commits ago. --- pkg/commands/git_commands/rebase.go | 14 +++++++++++++- .../interactive_rebase/reword_merge_commit.go | 6 ------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 7abd50027..cd7f4af11 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -400,7 +400,19 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er func (self *RebaseCommands) BeginInteractiveRebaseForCommit( commits []*models.Commit, commitIndex int, keepCommitsThatBecomeEmpty bool, ) error { - return self.BeginInteractiveRebaseForCommitRange(commits, commitIndex, commitIndex, keepCommitsThatBecomeEmpty) + if commitIndex < len(commits) && commits[commitIndex].IsMerge() { + if self.config.NeedsGpgSubprocessForCommit() { + return errors.New(self.Tr.DisabledForGPG) + } + + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ + baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex), + instruction: daemon.NewInsertBreakInstruction(), + keepCommitsThatBecomeEmpty: keepCommitsThatBecomeEmpty, + }).Run() + } else { + return self.BeginInteractiveRebaseForCommitRange(commits, commitIndex, commitIndex, keepCommitsThatBecomeEmpty) + } } func (self *RebaseCommands) BeginInteractiveRebaseForCommitRange( diff --git a/pkg/integration/tests/interactive_rebase/reword_merge_commit.go b/pkg/integration/tests/interactive_rebase/reword_merge_commit.go index e4c1ee210..6c0969066 100644 --- a/pkg/integration/tests/interactive_rebase/reword_merge_commit.go +++ b/pkg/integration/tests/interactive_rebase/reword_merge_commit.go @@ -40,17 +40,11 @@ var RewordMergeCommit = NewIntegrationTest(NewIntegrationTestArgs{ Type("renamed merge"). Confirm() }). - /* EXPECTED: Lines( Contains("CI ◯ two"), Contains("CI ⏣─╮ renamed merge").IsSelected(), Contains("CI │ ◯ one"), Contains("CI ◯ ╯ base"), ) - ACTUAL: */ - Tap(func() { - t.ExpectPopup().Alert().Title(Equals("Error")). - Content(Contains("error: 'edit' does not accept merge commits")) - }) }, })