diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index cd6033b39..d16728dc0 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -285,15 +285,14 @@ func (self *CommitCommands) ShowFileContentCmdObj(hash string, filePath string) return self.cmd.New(cmdArgs).DontLog() } -// Revert reverts the selected commit by hash -func (self *CommitCommands) Revert(hash string) error { - cmdArgs := NewGitCmd("revert").Arg(hash).ToArgv() - - return self.cmd.New(cmdArgs).Run() -} - -func (self *CommitCommands) RevertMerge(hash string, parentNumber int) error { - cmdArgs := NewGitCmd("revert").Arg(hash, "-m", fmt.Sprintf("%d", parentNumber)). +// Revert reverts the selected commit by hash. If isMerge is true, we'll pass -m 1 +// to say we want to revert the first parent of the merge commit, which is the one +// people want in 99.9% of cases. In current git versions we could unconditionally +// pass -m 1 even for non-merge commits, but older versions of git choke on it. +func (self *CommitCommands) Revert(hash string, isMerge bool) error { + cmdArgs := NewGitCmd("revert"). + Arg(hash). + ArgIf(isMerge, "-m", "1"). ToArgv() return self.cmd.New(cmdArgs).Run() diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 956482720..601ed32d2 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -1,7 +1,6 @@ package controllers import ( - "fmt" "strings" "github.com/go-errors/errors" @@ -859,10 +858,6 @@ func (self *LocalCommitsController) addCoAuthor(start, end int) error { } func (self *LocalCommitsController) revert(commit *models.Commit) error { - if commit.IsMerge() { - return self.createRevertMergeCommitMenu(commit) - } - self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.Actions.RevertCommit, Prompt: utils.ResolvePlaceholderString( @@ -873,7 +868,7 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error { HandleConfirm: func() error { self.c.LogAction(self.c.Tr.Actions.RevertCommit) return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - result := self.c.Git().Commit.Revert(commit.Hash) + result := self.c.Git().Commit.Revert(commit.Hash, commit.IsMerge()) if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil { return err } @@ -885,32 +880,6 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error { return nil } -func (self *LocalCommitsController) createRevertMergeCommitMenu(commit *models.Commit) error { - menuItems := make([]*types.MenuItem, len(commit.Parents)) - for i, parentHash := range commit.Parents { - message, err := self.c.Git().Commit.GetCommitMessageFirstLine(parentHash) - if err != nil { - return err - } - - menuItems[i] = &types.MenuItem{ - Label: fmt.Sprintf("%s: %s", utils.SafeTruncate(parentHash, 8), message), - OnPress: func() error { - parentNumber := i + 1 - self.c.LogAction(self.c.Tr.Actions.RevertCommit) - return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - if err := self.c.Git().Commit.RevertMerge(commit.Hash, parentNumber); err != nil { - return err - } - return self.afterRevertCommit() - }) - }, - } - } - - return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.SelectParentCommitForMerge, Items: menuItems}) -} - func (self *LocalCommitsController) afterRevertCommit() error { self.context().MoveSelection(1) return self.c.Refresh(types.RefreshOptions{ diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 5642e696f..eb9cd8faa 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -727,7 +727,6 @@ type TranslationSet struct { FocusCommandLog string CommandLogHeader string RandomTip string - SelectParentCommitForMerge string ToggleWhitespaceInDiffView string ToggleWhitespaceInDiffViewTooltip string IgnoreWhitespaceDiffViewSubTitle string @@ -1795,7 +1794,6 @@ func EnglishTranslationSet() *TranslationSet { FocusCommandLog: "Focus command log", CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n", RandomTip: "Random tip", - SelectParentCommitForMerge: "Select parent commit for merge", ToggleWhitespaceInDiffView: "Toggle whitespace", ToggleWhitespaceInDiffViewTooltip: "Toggle whether or not whitespace changes are shown in the diff view.", IgnoreWhitespaceDiffViewSubTitle: "(ignoring whitespace)", diff --git a/pkg/integration/tests/commit/revert_merge.go b/pkg/integration/tests/commit/revert_merge.go index 5d27970d0..1d4518086 100644 --- a/pkg/integration/tests/commit/revert_merge.go +++ b/pkg/integration/tests/commit/revert_merge.go @@ -21,14 +21,9 @@ var RevertMerge = NewIntegrationTest(NewIntegrationTestArgs{ ). Press(keys.Commits.RevertCommit) - t.ExpectPopup().Menu(). - Title(Equals("Select parent commit for merge")). - Lines( - Contains("first change"), - Contains("second-change-branch unrelated change"), - Contains("Cancel"), - ). - Select(Contains("first change")). + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). Confirm() t.Views().Commits().IsFocused().