Skip to content

Commit 06581e1

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/[email protected]/ for some discussion.
1 parent d81bb8b commit 06581e1

File tree

4 files changed

+12
-51
lines changed

4 files changed

+12
-51
lines changed

Diff for: pkg/commands/git_commands/commit.go

+8-9
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()

Diff for: pkg/gui/controllers/local_commits_controller.go

+1-32
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{

Diff for: pkg/i18n/english.go

-2
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,6 @@ type TranslationSet struct {
716716
FocusCommandLog string
717717
CommandLogHeader string
718718
RandomTip string
719-
SelectParentCommitForMerge string
720719
ToggleWhitespaceInDiffView string
721720
ToggleWhitespaceInDiffViewTooltip string
722721
IgnoreWhitespaceDiffViewSubTitle string
@@ -1770,7 +1769,6 @@ func EnglishTranslationSet() *TranslationSet {
17701769
FocusCommandLog: "Focus command log",
17711770
CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n",
17721771
RandomTip: "Random tip",
1773-
SelectParentCommitForMerge: "Select parent commit for merge",
17741772
ToggleWhitespaceInDiffView: "Toggle whitespace",
17751773
ToggleWhitespaceInDiffViewTooltip: "Toggle whether or not whitespace changes are shown in the diff view.",
17761774
IgnoreWhitespaceDiffViewSubTitle: "(ignoring whitespace)",

Diff for: pkg/integration/tests/commit/revert_merge.go

+3-8
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)