Add no-ff merge option

pkg/commands/git_commands/branch.go

@@ -250,26 +250,51 @@ 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()
 
 	return self.cmd.New(cmdArgs).Run()
 }
 
+// Returns whether refName can be fast-forward merged into the current branch
+func (self *BranchCommands) CanDoFastForwardMerge(refName string) bool {
+	cmdArgs := NewGitCmd("merge-base").
+		Arg("--is-ancestor").
+		Arg("HEAD", refName).
+		ToArgv()
+	err := self.cmd.New(cmdArgs).DontLog().Run()
+	return err == nil
+}
+
 // Only choose between non-empty, non-identical commands
 func (self *BranchCommands) allBranchesLogCandidates() []string {
 	return lo.Uniq(lo.WithoutEmpty(self.UserConfig().Git.AllBranchesLogCmds))

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/commands/git_commands/config.go

@@ -97,6 +97,10 @@ func (self *ConfigCommands) GetRebaseUpdateRefs() bool {
 	return self.gitConfig.GetBool("rebase.updateRefs")
 }
 
+func (self *ConfigCommands) GetMergeFF() string {
+	return self.gitConfig.Get("merge.ff")
+}
+
 func (self *ConfigCommands) DropConfigCache() {
 	self.gitConfig.DropCache()
 }

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -381,38 +381,103 @@ func (self *MergeAndRebaseHelper) MergeRefIntoCheckedOutBranch(refName string) e
 		return errors.New(self.c.Tr.CantMergeBranchIntoItself)
 	}
 
+	wantFastForward, wantNonFastForward := self.fastForwardMergeUserPreference()
+	canFastForward := self.c.Git().Branch.CanDoFastForwardMerge(refName)
+
+	var firstRegularMergeItem *types.MenuItem
+	var secondRegularMergeItem *types.MenuItem
+	var fastForwardMergeItem *types.MenuItem
+
+	if !wantNonFastForward && (wantFastForward || canFastForward) {
+		firstRegularMergeItem = &types.MenuItem{
+			Label:   self.c.Tr.RegularMergeFastForward,
+			OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_REGULAR),
+			Key:     'm',
+			Tooltip: utils.ResolvePlaceholderString(
+				self.c.Tr.RegularMergeFastForwardTooltip,
+				map[string]string{
+					"checkedOutBranch": checkedOutBranchName,
+					"selectedBranch":   refName,
+				},
+			),
+		}
+		fastForwardMergeItem = firstRegularMergeItem
+
+		secondRegularMergeItem = &types.MenuItem{
+			Label:   self.c.Tr.RegularMergeNonFastForward,
+			OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_NON_FAST_FORWARD),
+			Key:     'n',
+			Tooltip: utils.ResolvePlaceholderString(
+				self.c.Tr.RegularMergeNonFastForwardTooltip,
+				map[string]string{
+					"checkedOutBranch": checkedOutBranchName,
+					"selectedBranch":   refName,
+				},
+			),
+		}
+	} else {
+		firstRegularMergeItem = &types.MenuItem{
+			Label:   self.c.Tr.RegularMergeNonFastForward,
+			OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_REGULAR),
+			Key:     'm',
+			Tooltip: utils.ResolvePlaceholderString(
+				self.c.Tr.RegularMergeNonFastForwardTooltip,
+				map[string]string{
+					"checkedOutBranch": checkedOutBranchName,
+					"selectedBranch":   refName,
+				},
+			),
+		}
+
+		secondRegularMergeItem = &types.MenuItem{
+			Label:   self.c.Tr.RegularMergeFastForward,
+			OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_FAST_FORWARD),
+			Key:     'f',
+			Tooltip: utils.ResolvePlaceholderString(
+				self.c.Tr.RegularMergeFastForwardTooltip,
+				map[string]string{
+					"checkedOutBranch": checkedOutBranchName,
+					"selectedBranch":   refName,
+				},
+			),
+		}
+		fastForwardMergeItem = secondRegularMergeItem
+	}
+
+	if !canFastForward {
+		fastForwardMergeItem.DisabledReason = &types.DisabledReason{
+			Text: utils.ResolvePlaceholderString(
+				self.c.Tr.CannotFastForwardMerge,
+				map[string]string{
+					"checkedOutBranch": checkedOutBranchName,
+					"selectedBranch":   refName,
+				},
+			),
+		}
+	}
+
 	return self.c.Menu(types.CreateMenuOptions{
 		Title: self.c.Tr.Merge,
 		Items: []*types.MenuItem{
+			firstRegularMergeItem,
+			secondRegularMergeItem,
 			{
-				Label:   self.c.Tr.RegularMerge,
-				OnPress: self.RegularMerge(refName),
-				Key:     'm',
-				Tooltip: utils.ResolvePlaceholderString(
-					self.c.Tr.RegularMergeTooltip,
-					map[string]string{
-						"checkedOutBranch": checkedOutBranchName,
-						"selectedBranch":   refName,
-					},
-				),
-			},
-			{
-				Label:   self.c.Tr.SquashMergeUncommittedTitle,
+				Label:   self.c.Tr.SquashMergeUncommitted,
 				OnPress: self.SquashMergeUncommitted(refName),
 				Key:     's',
 				Tooltip: utils.ResolvePlaceholderString(
-					self.c.Tr.SquashMergeUncommitted,
+					self.c.Tr.SquashMergeUncommittedTooltip,
 					map[string]string{
 						"selectedBranch": refName,
 					},
 				),
 			},
 			{
-				Label:   self.c.Tr.SquashMergeCommittedTitle,
+				Label:   self.c.Tr.SquashMergeCommitted,
 				OnPress: self.SquashMergeCommitted(refName, checkedOutBranchName),
 				Key:     'S',
 				Tooltip: utils.ResolvePlaceholderString(
-					self.c.Tr.SquashMergeCommitted,
+					self.c.Tr.SquashMergeCommittedTooltip,
 					map[string]string{
 						"checkedOutBranch": checkedOutBranchName,
 						"selectedBranch":   refName,
@@ -423,26 +488,26 @@ func (self *MergeAndRebaseHelper) MergeRefIntoCheckedOutBranch(refName string) e
 	})
 }
 
-func (self *MergeAndRebaseHelper) RegularMerge(refName string) func() error {
+func (self *MergeAndRebaseHelper) RegularMerge(refName string, variant git_commands.MergeVariant) 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, variant)
 		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
 		}
@@ -459,6 +524,31 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch
 	}
 }
 
+// Returns wantsFastForward, wantsNonFastForward. These will never both be true, but they can both be false.
+func (self *MergeAndRebaseHelper) fastForwardMergeUserPreference() (bool, bool) {
+	// Check user config first, because it takes precedence over git config
+	mergingArgs := self.c.UserConfig().Git.Merging.Args
+	if strings.Contains(mergingArgs, "--ff") { // also covers "--ff-only"
+		return true, false
+	}
+
+	if strings.Contains(mergingArgs, "--no-ff") {
+		return false, true
+	}
+
+	// Then check git config
+	mergeFfConfig := self.c.Git().Config.GetMergeFF()
+	if mergeFfConfig == "true" || mergeFfConfig == "only" {
+		return true, false
+	}
+
+	if mergeFfConfig == "false" {
+		return false, true
+	}
+
+	return false, false
+}
+
 func (self *MergeAndRebaseHelper) ResetMarkedBaseCommit() error {
 	self.c.Modes().MarkedBaseCommit.Reset()
 	self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)

pkg/i18n/english.go

@@ -23,11 +23,6 @@ type TranslationSet struct {
 	StagedChanges                         string
 	StagingTitle                          string
 	MergingTitle                          string
-	SquashMergeUncommittedTitle           string
-	SquashMergeCommittedTitle             string
-	SquashMergeUncommitted                string
-	SquashMergeCommitted                  string
-	RegularMergeTooltip                   string
 	NormalTitle                           string
 	LogTitle                              string
 	LogXOfYTitle                          string
@@ -268,8 +263,16 @@ type TranslationSet struct {
 	RefreshFiles                          string
 	FocusMainView                         string
 	Merge                                 string
-	RegularMerge                          string
 	MergeBranchTooltip                    string
+	RegularMergeFastForward               string
+	RegularMergeFastForwardTooltip        string
+	CannotFastForwardMerge                string
+	RegularMergeNonFastForward            string
+	RegularMergeNonFastForwardTooltip     string
+	SquashMergeUncommitted                string
+	SquashMergeUncommittedTooltip         string
+	SquashMergeCommitted                  string
+	SquashMergeCommittedTooltip           string
 	ConfirmQuit                           string
 	SwitchRepo                            string
 	AllBranchesLogGraph                   string
@@ -1108,8 +1111,6 @@ func EnglishTranslationSet() *TranslationSet {
 		EasterEgg:                            "Easter egg",
 		UnstagedChanges:                      "Unstaged changes",
 		StagedChanges:                        "Staged changes",
-		SquashMergeUncommittedTitle:          "Squash merge and leave uncommitted",
-		SquashMergeCommittedTitle:            "Squash merge and commit",
 		StagingTitle:                         "Main panel (staging)",
 		MergingTitle:                         "Main panel (merging)",
 		NormalTitle:                          "Main panel (normal)",
@@ -1352,8 +1353,16 @@ func EnglishTranslationSet() *TranslationSet {
 		RefreshFiles:                         `Refresh files`,
 		FocusMainView:                        "Focus main view",
 		Merge:                                `Merge`,
-		RegularMerge:                         "Regular merge",
 		MergeBranchTooltip:                   "View options for merging the selected item into the current branch (regular merge, squash merge)",
+		RegularMergeFastForward:              "Regular merge (fast-forward)",
+		RegularMergeFastForwardTooltip:       "Fast-forward '{{.checkedOutBranch}}' to '{{.selectedBranch}}' without creating a merge commit.",
+		CannotFastForwardMerge:               "Cannot fast-forward '{{.checkedOutBranch}}' to '{{.selectedBranch}}'",
+		RegularMergeNonFastForward:           "Regular merge (with merge commit)",
+		RegularMergeNonFastForwardTooltip:    "Merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}', creating a merge commit.",
+		SquashMergeUncommitted:               "Squash merge and leave uncommitted",
+		SquashMergeUncommittedTooltip:        "Squash merge '{{.selectedBranch}}' into the working tree.",
+		SquashMergeCommitted:                 "Squash merge and commit",
+		SquashMergeCommittedTooltip:          "Squash merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}' as a single commit.",
 		ConfirmQuit:                          `Are you sure you want to quit?`,
 		SwitchRepo:                           `Switch to a recent repo`,
 		AllBranchesLogGraph:                  `Show/cycle all branch logs`,
@@ -1438,9 +1447,6 @@ func EnglishTranslationSet() *TranslationSet {
 		InteractiveRebaseTooltip:             "Begin an interactive rebase with a break at the start, so you can update the TODO commits before continuing.",
 		RebaseOntoBaseBranchTooltip:          "Rebase the checked out branch onto its base branch (i.e. the closest main branch).",
 		MustSelectTodoCommits:                "When rebasing, this action only works on a selection of TODO commits.",
-		SquashMergeUncommitted:               "Squash merge '{{.selectedBranch}}' into the working tree.",
-		SquashMergeCommitted:                 "Squash merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}' as a single commit.",
-		RegularMergeTooltip:                  "Merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}'.",
 		FwdNoUpstream:                        "Cannot fast-forward a branch with no upstream",
 		FwdNoLocalUpstream:                   "Cannot fast-forward a branch whose remote is not registered locally",
 		FwdCommitsToPush:                     "Cannot fast-forward a branch with commits to push",