drop merge commits when interactive rebasing just like git CLI

pkg/commands/commit.go

@@ -12,6 +12,9 @@ type Commit struct {
 	ExtraInfo     string // something like 'HEAD -> master, tag: v0.15.2'
 	Author        string
 	UnixTimestamp int64
+
+	// IsMerge tells us whether we're dealing with a merge commit i.e. a commit with two parents
+	IsMerge bool
 }
 
 func (c *Commit) ShortSha() string {

pkg/commands/commit_list_builder.go

@@ -57,7 +57,9 @@ func (c *CommitListBuilder) extractCommitFromLine(line string) *Commit {
 	unixTimestamp := split[1]
 	author := split[2]
 	extraInfo := strings.TrimSpace(split[3])
-	message := strings.Join(split[4:], SEPARATION_CHAR)
+	parentHashes := split[4]
+
+	message := strings.Join(split[5:], SEPARATION_CHAR)
 	tags := []string{}
 
 	if extraInfo != "" {
@@ -70,6 +72,10 @@ func (c *CommitListBuilder) extractCommitFromLine(line string) *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 &Commit{
 		Sha:           sha,
 		Name:          message,
@@ -77,6 +83,7 @@ func (c *CommitListBuilder) extractCommitFromLine(line string) *Commit {
 		ExtraInfo:     extraInfo,
 		UnixTimestamp: int64(unitTimestampInt),
 		Author:        author,
+		IsMerge:       isMerge,
 	}
 }
 
@@ -321,5 +328,18 @@ func (c *CommitListBuilder) getLogCmd(opts GetCommitsOptions) *exec.Cmd {
 		filterFlag = fmt.Sprintf(" --follow -- %s", c.OSCommand.Quote(opts.FilterPath))
 	}
 
-	return c.OSCommand.ExecutableFromString(fmt.Sprintf("git log %s --oneline --pretty=format:\"%%H%s%%at%s%%aN%s%%d%s%%s\" %s --abbrev=%d --date=unix %s", opts.RefName, SEPARATION_CHAR, SEPARATION_CHAR, SEPARATION_CHAR, SEPARATION_CHAR, limitFlag, 20, filterFlag))
+	return c.OSCommand.ExecutableFromString(
+		fmt.Sprintf(
+			"git log %s --oneline --pretty=format:\"%%H%s%%at%s%%aN%s%%d%s%%p%s%%s\" %s --abbrev=%d --date=unix %s",
+			opts.RefName,
+			SEPARATION_CHAR,
+			SEPARATION_CHAR,
+			SEPARATION_CHAR,
+			SEPARATION_CHAR,
+			SEPARATION_CHAR,
+			limitFlag,
+			20,
+			filterFlag,
+		),
+	)
 }

pkg/commands/git.go

@@ -910,7 +910,8 @@ func (c *GitCommand) PrepareInteractiveRebaseCommand(baseSha string, todo string
 		debug = "TRUE"
 	}
 
-	splitCmd := str.ToArgv(fmt.Sprintf("git rebase --interactive --autostash --keep-empty --rebase-merges %s", baseSha))
+	cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty %s", baseSha)
+	splitCmd := str.ToArgv(cmdStr)
 
 	cmd := c.OSCommand.command(splitCmd[0], splitCmd[1:]...)
 
@@ -962,11 +963,18 @@ func (c *GitCommand) GenerateGenericRebaseTodo(commits []*Commit, actionIndex in
 
 	todo := ""
 	for i, commit := range commits[0:baseIndex] {
-		a := "pick"
+		var commitAction string
 		if i == actionIndex {
-			a = action
+			commitAction = action
+		} 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.
+			commitAction = "drop"
+		} else {
+			commitAction = "pick"
 		}
-		todo = a + " " + commit.Sha + " " + commit.Name + "\n" + todo
+		todo = commitAction + " " + commit.Sha + " " + commit.Name + "\n" + todo
 	}
 
 	return todo, commits[baseIndex].Sha, nil

pkg/commands/git_test.go

@@ -1639,7 +1639,7 @@ func TestGitCommandRebaseBranch(t *testing.T) {
 			"master",
 			test.CreateMockCommand(t, []*test.CommandSwapper{
 				{
-					Expect:  "git rebase --interactive --autostash --keep-empty --rebase-merges master",
+					Expect:  "git rebase --interactive --autostash --keep-empty master",
 					Replace: "echo",
 				},
 			}),
@@ -1652,7 +1652,7 @@ func TestGitCommandRebaseBranch(t *testing.T) {
 			"master",
 			test.CreateMockCommand(t, []*test.CommandSwapper{
 				{
-					Expect:  "git rebase --interactive --autostash --keep-empty --rebase-merges master",
+					Expect:  "git rebase --interactive --autostash --keep-empty master",
 					Replace: "test",
 				},
 			}),
@@ -1775,7 +1775,7 @@ func TestGitCommandDiscardOldFileChanges(t *testing.T) {
 			"test999.txt",
 			test.CreateMockCommand(t, []*test.CommandSwapper{
 				{
-					Expect:  "git rebase --interactive --autostash --keep-empty --rebase-merges abcdef",
+					Expect:  "git rebase --interactive --autostash --keep-empty abcdef",
 					Replace: "echo",
 				},
 				{

pkg/gui/presentation/commits.go

@@ -32,7 +32,6 @@ func getFullDescriptionDisplayStringsForCommit(c *commands.Commit, cherryPickedC
 	yellow := color.New(color.FgYellow)
 	green := color.New(color.FgGreen)
 	blue := color.New(color.FgBlue)
-	cyan := color.New(color.FgCyan)
 	defaultColor := color.New(theme.DefaultTextColor)
 	diffedColor := color.New(theme.DiffTerminalColor)
 
@@ -66,7 +65,7 @@ func getFullDescriptionDisplayStringsForCommit(c *commands.Commit, cherryPickedC
 	tagString := ""
 	secondColumnString := blue.Sprint(utils.UnixToDate(c.UnixTimestamp))
 	if c.Action != "" {
-		secondColumnString = cyan.Sprint(c.Action)
+		secondColumnString = color.New(actionColorMap(c.Action)).Sprint(c.Action)
 	} else if c.ExtraInfo != "" {
 		tagColor := color.New(color.FgMagenta, color.Bold)
 		tagString = utils.ColoredStringDirect(c.ExtraInfo, tagColor) + " "
@@ -82,7 +81,6 @@ func getDisplayStringsForCommit(c *commands.Commit, cherryPickedCommitShaMap map
 	yellow := color.New(color.FgYellow)
 	green := color.New(color.FgGreen)
 	blue := color.New(color.FgBlue)
-	cyan := color.New(color.FgCyan)
 	defaultColor := color.New(theme.DefaultTextColor)
 	diffedColor := color.New(theme.DiffTerminalColor)
 
@@ -116,7 +114,7 @@ func getDisplayStringsForCommit(c *commands.Commit, cherryPickedCommitShaMap map
 	actionString := ""
 	tagString := ""
 	if c.Action != "" {
-		actionString = cyan.Sprint(utils.WithPadding(c.Action, 7)) + " "
+		actionString = color.New(actionColorMap(c.Action)).Sprint(utils.WithPadding(c.Action, 7)) + " "
 	} else if len(c.Tags) > 0 {
 		tagColor := color.New(color.FgMagenta, color.Bold)
 		tagString = utils.ColoredStringDirect(strings.Join(c.Tags, " "), tagColor) + " "
@@ -124,3 +122,18 @@ func getDisplayStringsForCommit(c *commands.Commit, cherryPickedCommitShaMap map
 
 	return []string{shaColor.Sprint(c.ShortSha()), actionString + tagString + defaultColor.Sprint(c.Name)}
 }
+
+func actionColorMap(str string) color.Attribute {
+	switch str {
+	case "pick":
+		return color.FgCyan
+	case "drop":
+		return color.FgRed
+	case "edit":
+		return color.FgGreen
+	case "fixup":
+		return color.FgMagenta
+	default:
+		return color.FgYellow
+	}
+}