support reverting merge commits

8 files changed, 102 insertions, 10 deletions

diff --git a/pkg/commands/commits.go b/pkg/commands/commits.go
index df1a8a2c7..23a2f99a5 100644
--- a/pkg/commands/commits.go
+++ b/pkg/commands/commits.go
@@ -47,6 +47,10 @@ func (c *GitCommand) GetCommitMessage(commitSha string) (string, error) {
 	return strings.TrimSpace(message), err
 }
 
+func (c *GitCommand) GetCommitMessageFirstLine(sha string) (string, error) {
+	return c.RunCommandWithOutput("git show --no-patch --pretty=format:%%s %s", sha)
+}
+
 // AmendHead amends HEAD with whatever is staged in your working tree
 func (c *GitCommand) AmendHead() error {
 	return c.OSCommand.RunCommand(c.AmendHeadCmdStr())
@@ -69,6 +73,10 @@ func (c *GitCommand) Revert(sha string) error {
 	return c.RunCommand("git revert %s", sha)
 }
 
+func (c *GitCommand) RevertMerge(sha string, parentNumber int) error {
+	return c.RunCommand("git revert %s -m %d", sha, parentNumber)
+}
+
 // CherryPickCommits begins an interactive rebase with the given shas being cherry picked onto HEAD
 func (c *GitCommand) CherryPickCommits(commits []*models.Commit) error {
 	todo := ""
diff --git a/pkg/commands/loading_commits.go b/pkg/commands/loading_commits.go
index bb25f4c62..bf7e4cf56 100644
--- a/pkg/commands/loading_commits.go
+++ b/pkg/commands/loading_commits.go
@@ -72,10 +72,6 @@ func (c *CommitListBuilder) extractCommitFromLine(line string) *models.Commit {
 
 	unitTimestampInt, _ := strconv.Atoi(unixTimestamp)
 
-	// Any commit with multiple parents is a merge commit.
-	// If there's a space then it means there must be more than one parent hash
-	isMerge := strings.Contains(parentHashes, " ")
-
 	return &models.Commit{
 		Sha:           sha,
 		Name:          message,
@@ -83,7 +79,7 @@ func (c *CommitListBuilder) extractCommitFromLine(line string) *models.Commit {
 		ExtraInfo:     extraInfo,
 		UnixTimestamp: int64(unitTimestampInt),
 		Author:        author,
-		IsMerge:       isMerge,
+		Parents:       strings.Split(parentHashes, " "),
 	}
 }
 
diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go
index 78cf7f7f7..1b8659e77 100644
--- a/pkg/commands/models/commit.go
+++ b/pkg/commands/models/commit.go
@@ -13,8 +13,8 @@ type Commit struct {
 	Author        string
 	UnixTimestamp int64
 
-	// IsMerge tells us whether we're dealing with a merge commit i.e. a commit with two parents
-	IsMerge bool
+	// SHAs of parent commits (will be multiple if it's a merge commit)
+	Parents []string
 }
 
 func (c *Commit) ShortSha() string {
@@ -35,3 +35,7 @@ func (c *Commit) ID() string {
 func (c *Commit) Description() string {
 	return fmt.Sprintf("%s %s", c.Sha[:7], c.Name)
 }
+
+func (c *Commit) IsMerge() bool {
+	return len(c.Parents) > 1
+}
diff --git a/pkg/commands/rebasing.go b/pkg/commands/rebasing.go
index ca0b432e2..63c568036 100644
--- a/pkg/commands/rebasing.go
+++ b/pkg/commands/rebasing.go
@@ -119,7 +119,7 @@ func (c *GitCommand) GenerateGenericRebaseTodo(commits []*models.Commit, actionI
 		var commitAction string
 		if i == actionIndex {
 			commitAction = action
-		} else if commit.IsMerge {
+		} else if commit.IsMerge() {
 			// your typical interactive rebase will actually drop merge commits by default. Damn git CLI, you scary!
 			// doing this means we don't need to worry about rebasing over merges which always causes problems.
 			// you typically shouldn't be doing rebases that pass over merge commits anyway.
diff --git a/pkg/gui/commits_panel.go b/pkg/gui/commits_panel.go
index 01264948e..422d8ec29 100644
--- a/pkg/gui/commits_panel.go
+++ b/pkg/gui/commits_panel.go
@@ -461,9 +461,43 @@ func (gui *Gui) handleCommitRevert() error {
 		return err
 	}
 
-	if err := gui.GitCommand.WithSpan(gui.Tr.Spans.RevertCommit).Revert(gui.State.Commits[gui.State.Panels.Commits.SelectedLineIdx].Sha); err != nil {
-		return gui.surfaceError(err)
+	commit := gui.getSelectedLocalCommit()
+
+	if commit.IsMerge() {
+		return gui.createRevertMergeCommitMenu(commit)
+	} else {
+		if err := gui.GitCommand.WithSpan(gui.Tr.Spans.RevertCommit).Revert(commit.Sha); err != nil {
+			return gui.surfaceError(err)
+		}
+		return gui.afterRevertCommit()
+	}
+}
+
+func (gui *Gui) createRevertMergeCommitMenu(commit *models.Commit) error {
+	menuItems := make([]*menuItem, len(commit.Parents))
+	for i, parentSha := range commit.Parents {
+		i := i
+		message, err := gui.GitCommand.GetCommitMessageFirstLine(parentSha)
+		if err != nil {
+			return gui.surfaceError(err)
+		}
+
+		menuItems[i] = &menuItem{
+			displayString: fmt.Sprintf("%s: %s", utils.SafeTruncate(parentSha, 8), message),
+			onPress: func() error {
+				parentNumber := i + 1
+				if err := gui.GitCommand.WithSpan(gui.Tr.Spans.RevertCommit).RevertMerge(commit.Sha, parentNumber); err != nil {
+					return gui.surfaceError(err)
+				}
+				return gui.afterRevertCommit()
+			},
+		}
 	}
+
+	return gui.createMenu(gui.Tr.SelectParentCommitForMerge, menuItems, createMenuOptions{showCancel: true})
+}
+
+func (gui *Gui) afterRevertCommit() error {
 	gui.State.Panels.Commits.SelectedLineIdx++
 	return gui.refreshSidePanels(refreshOptions{mode: BLOCK_UI, scope: []RefreshableView{COMMITS, BRANCHES}})
 }
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 1fac1b322..317572b81 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -449,6 +449,7 @@ type TranslationSet struct {
 	FocusCommandLog                     string
 	CommandLogHeader                    string
 	RandomTip                           string
+	SelectParentCommitForMerge          string
 	Spans                               Spans
 }
 
@@ -991,6 +992,7 @@ func englishTranslationSet() TranslationSet {
 		FocusCommandLog:                     "Focus command log",
 		CommandLogHeader:                    "You can hide/focus this panel by pressing '%s' or hide it permanently in your config with `gui.showCommandLog: false`\n",
 		RandomTip:                           "Random Tip",
+		SelectParentCommitForMerge:          "Select parent commit for merge",
 		Spans: Spans{
 			// TODO: combine this with the original keybinding descriptions (those are all in lowercase atm)
 			CheckoutCommit:                    "Checkout commit",
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
index 4ec1c624c..7b5a3071c 100644
--- a/pkg/utils/utils.go
+++ b/pkg/utils/utils.go
@@ -87,6 +87,14 @@ func TruncateWithEllipsis(str string, limit int) string {
 	return str[0:remainingLength] + "..."
 }
 
+func SafeTruncate(str string, limit int) string {
+	if len(str) > limit {
+		return str[0:limit]
+	} else {
+		return str
+	}
+}
+
 func FindStringSubmatch(str string, regexpStr string) (bool, []string) {
 	re := regexp.MustCompile(regexpStr)
 	match := re.FindStringSubmatch(str)
diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
index c63028d41..02aded559 100644
--- a/pkg/utils/utils_test.go
+++ b/pkg/utils/utils_test.go
@@ -47,3 +47,43 @@ func TestAsJson(t *testing.T) {
 	// no idea why this is returning empty hashes but it's works in the app ¯\_(ツ)_/¯
 	assert.EqualValues(t, "{}", output)
 }
+
+func TestSafeTruncate(t *testing.T) {
+	type scenario struct {
+		str      string
+		limit    int
+		expected string
+	}
+
+	scenarios := []scenario{
+		{
+			str:      "",
+			limit:    0,
+			expected: "",
+		},
+		{
+			str:      "12345",
+			limit:    3,
+			expected: "123",
+		},
+		{
+			str:      "12345",
+			limit:    4,
+			expected: "1234",
+		},
+		{
+			str:      "12345",
+			limit:    5,
+			expected: "12345",
+		},
+		{
+			str:      "12345",
+			limit:    6,
+			expected: "12345",
+		},
+	}
+
+	for _, s := range scenarios {
+		assert.EqualValues(t, s.expected, SafeTruncate(s.str, s.limit))
+	}
+}