Replace MergeOpts struct with MergeVariant enum

- Squash and FastForwardOnly are mutually exclusive, and instead of asserting
  this at runtime, model the API so that they can't be passed together.
- FastForwardOnly is unused, so remove it; however, we are going to need --ff
  and --no-ff in the next commit, so add those instead.
- Instead of putting the enum into the MergeOpts struct, replace the struct by
  the enum. We can reintroduce the struct when we add more arguments, but for
  now it's an unnecessary indirection.

Author: stefanhaller

pkg/commands/git_commands/branch.go

@@ -250,20 +250,35 @@ func (self *BranchCommands) Rename(oldName string, newName string) error {
 	return self.cmd.New(cmdArgs).Run()
 }
 
-type MergeOpts struct {
-	FastForwardOnly bool
-	Squash          bool
-}
+type MergeVariant int
+
+const (
+	MERGE_VARIANT_REGULAR MergeVariant = iota
+	MERGE_VARIANT_FAST_FORWARD
+	MERGE_VARIANT_NON_FAST_FORWARD
+	MERGE_VARIANT_SQUASH
+)
+
+func (self *BranchCommands) Merge(branchName string, variant MergeVariant) error {
+	extraArgs := func() []string {
+		switch variant {
+		case MERGE_VARIANT_REGULAR:
+			return []string{}
+		case MERGE_VARIANT_FAST_FORWARD:
+			return []string{"--ff"}
+		case MERGE_VARIANT_NON_FAST_FORWARD:
+			return []string{"--no-ff"}
+		case MERGE_VARIANT_SQUASH:
+			return []string{"--squash", "--ff"}
+		}
+
+		panic("shouldn't get here")
+	}()
 
-func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
-	if opts.Squash && opts.FastForwardOnly {
-		panic("Squash and FastForwardOnly can't both be true")
-	}
 	cmdArgs := NewGitCmd("merge").
 		Arg("--no-edit").
 		Arg(strings.Fields(self.UserConfig().Git.Merging.Args)...).
-		ArgIf(opts.FastForwardOnly, "--ff-only").
-		ArgIf(opts.Squash, "--squash", "--ff").
+		Arg(extraArgs...).
 		Arg(branchName).
 		ToArgv()
 

pkg/commands/git_commands/branch_test.go

@@ -122,14 +122,14 @@ func TestBranchMerge(t *testing.T) {
 	scenarios := []struct {
 		testName   string
 		userConfig *config.UserConfig
-		opts       MergeOpts
+		variant    MergeVariant
 		branchName string
 		expected   []string
 	}{
 		{
 			testName:   "basic",
 			userConfig: &config.UserConfig{},
-			opts:       MergeOpts{},
+			variant:    MERGE_VARIANT_REGULAR,
 			branchName: "mybranch",
 			expected:   []string{"merge", "--no-edit", "mybranch"},
 		},
@@ -142,7 +142,7 @@ func TestBranchMerge(t *testing.T) {
 					},
 				},
 			},
-			opts:       MergeOpts{},
+			variant:    MERGE_VARIANT_REGULAR,
 			branchName: "mybranch",
 			expected:   []string{"merge", "--no-edit", "--merging-args", "mybranch"},
 		},
@@ -155,16 +155,30 @@ func TestBranchMerge(t *testing.T) {
 					},
 				},
 			},
-			opts:       MergeOpts{},
+			variant:    MERGE_VARIANT_REGULAR,
 			branchName: "mybranch",
 			expected:   []string{"merge", "--no-edit", "--arg1", "--arg2", "mybranch"},
 		},
 		{
-			testName:   "fast forward only",
+			testName:   "fast-forward merge",
 			userConfig: &config.UserConfig{},
-			opts:       MergeOpts{FastForwardOnly: true},
+			variant:    MERGE_VARIANT_FAST_FORWARD,
 			branchName: "mybranch",
-			expected:   []string{"merge", "--no-edit", "--ff-only", "mybranch"},
+			expected:   []string{"merge", "--no-edit", "--ff", "mybranch"},
+		},
+		{
+			testName:   "non-fast-forward merge",
+			userConfig: &config.UserConfig{},
+			variant:    MERGE_VARIANT_NON_FAST_FORWARD,
+			branchName: "mybranch",
+			expected:   []string{"merge", "--no-edit", "--no-ff", "mybranch"},
+		},
+		{
+			testName:   "squash merge",
+			userConfig: &config.UserConfig{},
+			variant:    MERGE_VARIANT_SQUASH,
+			branchName: "mybranch",
+			expected:   []string{"merge", "--no-edit", "--squash", "--ff", "mybranch"},
 		},
 	}
 
@@ -174,7 +188,7 @@ func TestBranchMerge(t *testing.T) {
 				ExpectGitArgs(s.expected, "", nil)
 			instance := buildBranchCommands(commonDeps{runner: runner, userConfig: s.userConfig})
 
-			assert.NoError(t, instance.Merge(s.branchName, s.opts))
+			assert.NoError(t, instance.Merge(s.branchName, s.variant))
 			runner.CheckForMissingCalls()
 		})
 	}

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -426,23 +426,23 @@ func (self *MergeAndRebaseHelper) MergeRefIntoCheckedOutBranch(refName string) e
 func (self *MergeAndRebaseHelper) RegularMerge(refName string) func() error {
 	return func() error {
 		self.c.LogAction(self.c.Tr.Actions.Merge)
-		err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{})
+		err := self.c.Git().Branch.Merge(refName, git_commands.MERGE_VARIANT_REGULAR)
 		return self.CheckMergeOrRebase(err)
 	}
 }
 
 func (self *MergeAndRebaseHelper) SquashMergeUncommitted(refName string) func() error {
 	return func() error {
 		self.c.LogAction(self.c.Tr.Actions.SquashMerge)
-		err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{Squash: true})
+		err := self.c.Git().Branch.Merge(refName, git_commands.MERGE_VARIANT_SQUASH)
 		return self.CheckMergeOrRebase(err)
 	}
 }
 
 func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranchName string) func() error {
 	return func() error {
 		self.c.LogAction(self.c.Tr.Actions.SquashMerge)
-		err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{Squash: true})
+		err := self.c.Git().Branch.Merge(refName, git_commands.MERGE_VARIANT_SQUASH)
 		if err = self.CheckMergeOrRebase(err); err != nil {
 			return err
 		}