support reverting merge commits

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 := ""

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, " "),
 	}
 }
 

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
+}

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.

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 {
+	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}})
 }

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",

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)

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))
+	}
+}

test/integration/mergeConflictRevert/expected/.git_keep/COMMIT_EDITMSG

@@ -0,0 +1 @@
+test

test/integration/mergeConflictRevert/expected/.git_keep/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/other

test/integration/mergeConflictRevert/expected/.git_keep/ORIG_HEAD

@@ -0,0 +1 @@
+e5265503c8aea2860fc4754c1025e4597530ce0e

test/integration/mergeConflictRevert/expected/.git_keep/config

@@ -0,0 +1,10 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+[user]
+	email = CI@example.com
+	name = CI

test/integration/mergeConflictRevert/expected/.git_keep/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

test/integration/mergeConflictRevert/expected/.git_keep/info/exclude

@@ -0,0 +1,7 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~
+.DS_Store

test/integration/mergeConflictRevert/expected/.git_keep/logs/HEAD

@@ -0,0 +1,12 @@
+0000000000000000000000000000000000000000 d3b35176a575d48743900b1f0863cefbc198f84c CI <CI@example.com> 1622875852 +1000	commit (initial): test 1
+d3b35176a575d48743900b1f0863cefbc198f84c d3b35176a575d48743900b1f0863cefbc198f84c CI <CI@example.com> 1622875853 +1000	checkout: moving from master to other
+d3b35176a575d48743900b1f0863cefbc198f84c 3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 CI <CI@example.com> 1622875853 +1000	commit: test 2
+3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 CI <CI@example.com> 1622875853 +1000	checkout: moving from other to another
+3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 1e7d643e0db24ebee10f92aa2f8099d50dbe0f0f CI <CI@example.com> 1622875853 +1000	commit: test 3
+1e7d643e0db24ebee10f92aa2f8099d50dbe0f0f 3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 CI <CI@example.com> 1622875853 +1000	checkout: moving from another to other
+3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 fa0b6bf64815f57729716334319596c926b6564a CI <CI@example.com> 1622875853 +1000	commit: test 4
+fa0b6bf64815f57729716334319596c926b6564a ba4581dc53b5b2ff56803651dfd79245203d546b CI <CI@example.com> 1622875853 +1000	merge another: Merge made by the 'recursive' strategy.
+ba4581dc53b5b2ff56803651dfd79245203d546b a4942a576eec3a1a15fb790c942b6860331bee32 CI <CI@example.com> 1622875853 +1000	commit: test 5
+a4942a576eec3a1a15fb790c942b6860331bee32 e5265503c8aea2860fc4754c1025e4597530ce0e CI <CI@example.com> 1622875856 +1000	revert: Revert "Merge branch 'another' into other"
+e5265503c8aea2860fc4754c1025e4597530ce0e a4942a576eec3a1a15fb790c942b6860331bee32 CI <CI@example.com> 1622875860 +1000	reset: moving to a4942a576eec3a1a15fb790c942b6860331bee32
+a4942a576eec3a1a15fb790c942b6860331bee32 ddad764e9e78b555cd41e5e81f8ce969cfa3972c CI <CI@example.com> 1622875862 +1000	commit: test

test/integration/mergeConflictRevert/expected/.git_keep/logs/refs/heads/another

@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 CI <CI@example.com> 1622875853 +1000	branch: Created from HEAD
+3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 1e7d643e0db24ebee10f92aa2f8099d50dbe0f0f CI <CI@example.com> 1622875853 +1000	commit: test 3

test/integration/mergeConflictRevert/expected/.git_keep/logs/refs/heads/master

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 d3b35176a575d48743900b1f0863cefbc198f84c CI <CI@example.com> 1622875852 +1000	commit (initial): test 1

test/integration/mergeConflictRevert/expected/.git_keep/logs/refs/heads/other

@@ -0,0 +1,8 @@
+0000000000000000000000000000000000000000 d3b35176a575d48743900b1f0863cefbc198f84c CI <CI@example.com> 1622875853 +1000	branch: Created from HEAD
+d3b35176a575d48743900b1f0863cefbc198f84c 3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 CI <CI@example.com> 1622875853 +1000	commit: test 2
+3c3594b2fd655fb7ffe36077ee8a9c3f79fb5fc6 fa0b6bf64815f57729716334319596c926b6564a CI <CI@example.com> 1622875853 +1000	commit: test 4
+fa0b6bf64815f57729716334319596c926b6564a ba4581dc53b5b2ff56803651dfd79245203d546b CI <CI@example.com> 1622875853 +1000	merge another: Merge made by the 'recursive' strategy.
+ba4581dc53b5b2ff56803651dfd79245203d546b a4942a576eec3a1a15fb790c942b6860331bee32 CI <CI@example.com> 1622875853 +1000	commit: test 5
+a4942a576eec3a1a15fb790c942b6860331bee32 e5265503c8aea2860fc4754c1025e4597530ce0e CI <CI@example.com> 1622875856 +1000	revert: Revert "Merge branch 'another' into other"
+e5265503c8aea2860fc4754c1025e4597530ce0e a4942a576eec3a1a15fb790c942b6860331bee32 CI <CI@example.com> 1622875860 +1000	reset: moving to a4942a576eec3a1a15fb790c942b6860331bee32
+a4942a576eec3a1a15fb790c942b6860331bee32 ddad764e9e78b555cd41e5e81f8ce969cfa3972c CI <CI@example.com> 1622875862 +1000	commit: test

test/integration/mergeConflictRevert/expected/.git_keep/objects/a4/942a576eec3a1a15fb790c942b6860331bee32

@@ -0,0 +1,2 @@
+x
�ŽK
+1]罤әÎDWsŒtÒƒ‚q†1‚Ç7GpóEQ¼²¶öè`“;ô]DY&/cIe¬(ù`±f*š´R	š-ïúê yâhka',´,ì#:϶.5$š˜ÐUA“?ý¾îp›á|›¯úÍm{ꩬíÖÅÀ‘-"šAÇ©®ê¦ë»›Ð)9Ñ

test/integration/mergeConflictRevert/expected/.git_keep/objects/dd/ad764e9e78b555cd41e5e81f8ce969cfa3972c

@@ -0,0 +1,2 @@
+x
�ÎM
+1@a×=E÷‚$mÓf5ÇHcŠ‚u†±‚ÇwŽàöã-ž,½?†Å‚‡±©Z!nÁ·(‚7B­Õs
À‡D…(`˘J³ò¦¯a9”à˜RTÏÈH­¦²k�9‚÷XU½3ü÷e³ÓlÏÓ|Õ/÷õ©'YúÅbt.'ÊÑÙ#€ÙuŸúgn†¾‡ù
ÿ8ê

test/integration/mergeConflictRevert/expected/.git_keep/objects/e5/265503c8aea2860fc4754c1025e4597530ce0e

@@ -0,0 +1 @@
+x
�PΛjΓ0μY_±δ’CKΠkW”RΘ)‡^J`%―bCmY-ύό�τz†y1e�穃#ύΠ›δκ]¥RΜ€Frv�}ΤΪω€	Ρ›MH©¨7Y:°Oή2)�
¬9$]v6S$ν�Ι"Ξ*ώκγΪΰ|�ησεU~xΎ}Κ©¬σ²6�Hπh΄ΦjgχQ]ώ)Wος-­ΓαMΪU 7^ΚG^Φ>J;Β΄τξψ ΤΗ8mΠξ†
ώ� ³Ηh†‚.c¶µ"EνΝP‡�¬G«έ€�ςΣ�q›–«*#/WΩ`ζA`Ο―¬3εJ>ξ`6CΞyg&*ΙR&$Ο'υΉl'

test/integration/mergeConflictRevert/expected/.git_keep/refs/heads/another

@@ -0,0 +1 @@
+1e7d643e0db24ebee10f92aa2f8099d50dbe0f0f

test/integration/mergeConflictRevert/expected/.git_keep/refs/heads/master

@@ -0,0 +1 @@
+d3b35176a575d48743900b1f0863cefbc198f84c

test/integration/mergeConflictRevert/expected/.git_keep/refs/heads/other

@@ -0,0 +1 @@
+ddad764e9e78b555cd41e5e81f8ce969cfa3972c

test/integration/mergeConflictRevert/expected/file1

@@ -0,0 +1 @@
+test

test/integration/mergeConflictRevert/expected/file2

@@ -0,0 +1 @@
+test

test/integration/mergeConflictRevert/expected/file4

@@ -0,0 +1 @@
+test

test/integration/mergeConflictRevert/expected/file5

@@ -0,0 +1 @@
+test

test/integration/mergeConflictRevert/recording.json

@@ -0,0 +1 @@
+{"KeyEvents":[{"Timestamp":1730,"Mod":0,"Key":259,"Ch":0},{"Timestamp":2002,"Mod":0,"Key":259,"Ch":0},{"Timestamp":2305,"Mod":0,"Key":258,"Ch":0},{"Timestamp":2968,"Mod":0,"Key":256,"Ch":116},{"Timestamp":3402,"Mod":0,"Key":13,"Ch":13},{"Timestamp":3985,"Mod":0,"Key":257,"Ch":0},{"Timestamp":4304,"Mod":0,"Key":257,"Ch":0},{"Timestamp":5850,"Mod":0,"Key":258,"Ch":0},{"Timestamp":6785,"Mod":0,"Key":256,"Ch":103},{"Timestamp":7441,"Mod":0,"Key":13,"Ch":13},{"Timestamp":7978,"Mod":0,"Key":260,"Ch":0},{"Timestamp":8304,"Mod":0,"Key":260,"Ch":0},{"Timestamp":8585,"Mod":0,"Key":256,"Ch":99},{"Timestamp":8816,"Mod":0,"Key":256,"Ch":116},{"Timestamp":8873,"Mod":0,"Key":256,"Ch":101},{"Timestamp":9041,"Mod":0,"Key":256,"Ch":115},{"Timestamp":9081,"Mod":0,"Key":256,"Ch":116},{"Timestamp":9416,"Mod":0,"Key":13,"Ch":13},{"Timestamp":11041,"Mod":0,"Key":256,"Ch":113}],"ResizeEvents":[{"Timestamp":0,"Width":135,"Height":74}]}

test/integration/mergeConflictRevert/setup.sh

@@ -0,0 +1,38 @@
+#!/bin/sh
+
+cd $1
+
+git init
+git config user.email "CI@example.com"
+git config user.name "CI"
+
+git checkout -b master
+
+echo "test" > file1
+git add .
+git commit -m "test 1"
+
+git checkout -b other
+
+echo "test" > file2
+git add .
+git commit -m "test 2"
+
+git checkout -b another
+
+echo "test" > file3
+git add .
+git commit -m "test 3"
+
+git checkout other
+
+echo "test" > file4
+git add .
+git commit -m "test 4"
+
+git merge another
+
+echo "test" > file5
+git add .
+git commit -m "test 5"
+

test/integration/mergeConflictRevert/test.json

@@ -0,0 +1 @@
+{ "description": "In this test we revert a merge conflict, choosing which parent we want to retain... or something like that. We need to rename the newly created commit so that we don't fail on the snapshot comparison", "speed": 10 }