Skip to content

Commit f14a3cd

Browse files
committed
Remove the "Select parent commit" prompt when reverting a merge commit
In pretty much 100% of the cases, you want to use -m1, so spare users the complexity of a confusing prompt. See https://public-inbox.org/git/e60a8b1a-98c8-4ac7-b966-ff9635bb781d@haller-berlin.de/ for some discussion.
1 parent c13c635 commit f14a3cd

File tree

4 files changed

+12
-51
lines changed

4 files changed

+12
-51
lines changed

pkg/commands/git_commands/commit.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -285,15 +285,14 @@ func (self *CommitCommands) ShowFileContentCmdObj(hash string, filePath string)
285285
return self.cmd.New(cmdArgs).DontLog()
286286
}
287287

288-
// Revert reverts the selected commit by hash
289-
func (self *CommitCommands) Revert(hash string) error {
290-
cmdArgs := NewGitCmd("revert").Arg(hash).ToArgv()
291-
292-
return self.cmd.New(cmdArgs).Run()
293-
}
294-
295-
func (self *CommitCommands) RevertMerge(hash string, parentNumber int) error {
296-
cmdArgs := NewGitCmd("revert").Arg(hash, "-m", fmt.Sprintf("%d", parentNumber)).
288+
// Revert reverts the selected commit by hash. If isMerge is true, we'll pass -m 1
289+
// to say we want to revert the first parent of the merge commit, which is the one
290+
// people want in 99.9% of cases. In current git versions we could unconditionally
291+
// pass -m 1 even for non-merge commits, but older versions of git choke on it.
292+
func (self *CommitCommands) Revert(hash string, isMerge bool) error {
293+
cmdArgs := NewGitCmd("revert").
294+
Arg(hash).
295+
ArgIf(isMerge, "-m", "1").
297296
ToArgv()
298297

299298
return self.cmd.New(cmdArgs).Run()

pkg/gui/controllers/local_commits_controller.go

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package controllers
22

33
import (
4-
"fmt"
54
"strings"
65

76
"github.com/go-errors/errors"
@@ -859,10 +858,6 @@ func (self *LocalCommitsController) addCoAuthor(start, end int) error {
859858
}
860859

861860
func (self *LocalCommitsController) revert(commit *models.Commit) error {
862-
if commit.IsMerge() {
863-
return self.createRevertMergeCommitMenu(commit)
864-
}
865-
866861
self.c.Confirm(types.ConfirmOpts{
867862
Title: self.c.Tr.Actions.RevertCommit,
868863
Prompt: utils.ResolvePlaceholderString(
@@ -873,7 +868,7 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error {
873868
HandleConfirm: func() error {
874869
self.c.LogAction(self.c.Tr.Actions.RevertCommit)
875870
return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error {
876-
result := self.c.Git().Commit.Revert(commit.Hash)
871+
result := self.c.Git().Commit.Revert(commit.Hash, commit.IsMerge())
877872
if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil {
878873
return err
879874
}
@@ -885,32 +880,6 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error {
885880
return nil
886881
}
887882

888-
func (self *LocalCommitsController) createRevertMergeCommitMenu(commit *models.Commit) error {
889-
menuItems := make([]*types.MenuItem, len(commit.Parents))
890-
for i, parentHash := range commit.Parents {
891-
message, err := self.c.Git().Commit.GetCommitMessageFirstLine(parentHash)
892-
if err != nil {
893-
return err
894-
}
895-
896-
menuItems[i] = &types.MenuItem{
897-
Label: fmt.Sprintf("%s: %s", utils.SafeTruncate(parentHash, 8), message),
898-
OnPress: func() error {
899-
parentNumber := i + 1
900-
self.c.LogAction(self.c.Tr.Actions.RevertCommit)
901-
return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error {
902-
if err := self.c.Git().Commit.RevertMerge(commit.Hash, parentNumber); err != nil {
903-
return err
904-
}
905-
return self.afterRevertCommit()
906-
})
907-
},
908-
}
909-
}
910-
911-
return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.SelectParentCommitForMerge, Items: menuItems})
912-
}
913-
914883
func (self *LocalCommitsController) afterRevertCommit() error {
915884
self.context().MoveSelection(1)
916885
return self.c.Refresh(types.RefreshOptions{

pkg/i18n/english.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,6 @@ type TranslationSet struct {
727727
FocusCommandLog string
728728
CommandLogHeader string
729729
RandomTip string
730-
SelectParentCommitForMerge string
731730
ToggleWhitespaceInDiffView string
732731
ToggleWhitespaceInDiffViewTooltip string
733732
IgnoreWhitespaceDiffViewSubTitle string
@@ -1795,7 +1794,6 @@ func EnglishTranslationSet() *TranslationSet {
17951794
FocusCommandLog: "Focus command log",
17961795
CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n",
17971796
RandomTip: "Random tip",
1798-
SelectParentCommitForMerge: "Select parent commit for merge",
17991797
ToggleWhitespaceInDiffView: "Toggle whitespace",
18001798
ToggleWhitespaceInDiffViewTooltip: "Toggle whether or not whitespace changes are shown in the diff view.",
18011799
IgnoreWhitespaceDiffViewSubTitle: "(ignoring whitespace)",

pkg/integration/tests/commit/revert_merge.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,9 @@ var RevertMerge = NewIntegrationTest(NewIntegrationTestArgs{
2121
).
2222
Press(keys.Commits.RevertCommit)
2323

24-
t.ExpectPopup().Menu().
25-
Title(Equals("Select parent commit for merge")).
26-
Lines(
27-
Contains("first change"),
28-
Contains("second-change-branch unrelated change"),
29-
Contains("Cancel"),
30-
).
31-
Select(Contains("first change")).
24+
t.ExpectPopup().Confirmation().
25+
Title(Equals("Revert commit")).
26+
Content(MatchesRegexp(`Are you sure you want to revert \w+?`)).
3227
Confirm()
3328

3429
t.Views().Commits().IsFocused().

0 commit comments

Comments
 (0)