diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go index 13abbd69744..cd78a755b7f 100644 --- a/pkg/commands/git_commands/branch.go +++ b/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)) diff --git a/pkg/commands/git_commands/branch_test.go b/pkg/commands/git_commands/branch_test.go index 37ad79613de..a0c0096b945 100644 --- a/pkg/commands/git_commands/branch_test.go +++ b/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() }) } diff --git a/pkg/commands/git_commands/config.go b/pkg/commands/git_commands/config.go index fa657035693..a9fd4e14772 100644 --- a/pkg/commands/git_commands/config.go +++ b/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() } diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 88166ec4951..7d4f0a96a7d 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/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,10 +488,10 @@ 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) } } @@ -434,7 +499,7 @@ func (self *MergeAndRebaseHelper) RegularMerge(refName string) func() error { 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) } } @@ -442,7 +507,7 @@ func (self *MergeAndRebaseHelper) SquashMergeUncommitted(refName string) func() 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) diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 6c474c0b049..cb070b89207 100644 --- a/pkg/i18n/english.go +++ b/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", diff --git a/pkg/integration/tests/branch/merge_fast_forward.go b/pkg/integration/tests/branch/merge_fast_forward.go new file mode 100644 index 00000000000..707af1b3e8a --- /dev/null +++ b/pkg/integration/tests/branch/merge_fast_forward.go @@ -0,0 +1,68 @@ +package branch + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MergeFastForward = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Merge a branch into another using fast-forward merge", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "alphabetical" + }, + SetupRepo: func(shell *Shell) { + shell.NewBranch("original-branch"). + EmptyCommit("one"). + NewBranch("branch1"). + EmptyCommit("branch1"). + Checkout("original-branch"). + NewBranchFrom("branch2", "original-branch"). + EmptyCommit("branch2"). + Checkout("original-branch") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("original-branch").IsSelected(), + Contains("branch1"), + Contains("branch2"), + ). + SelectNextItem(). + Press(keys.Branches.MergeIntoCurrentBranch) + + t.ExpectPopup().Menu(). + Title(Equals("Merge")). + TopLines( + Contains("Regular merge (fast-forward)"), + Contains("Regular merge (with merge commit)"), + ). + Select(Contains("Regular merge (fast-forward)")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("branch1").IsSelected(), + Contains("one"), + ) + + // Check that branch2 can't be merged using fast-forward + t.Views().Branches(). + Focus(). + NavigateToLine(Contains("branch2")). + Press(keys.Branches.MergeIntoCurrentBranch) + + t.ExpectPopup().Menu(). + Title(Equals("Merge")). + TopLines( + Contains("Regular merge (with merge commit)"), + Contains("Regular merge (fast-forward)"), + ). + Select(Contains("Regular merge (fast-forward)")). + Confirm() + + t.ExpectToast(Contains("Cannot fast-forward 'original-branch' to 'branch2'")) + }, +}) diff --git a/pkg/integration/tests/branch/merge_non_fast_forward.go b/pkg/integration/tests/branch/merge_non_fast_forward.go new file mode 100644 index 00000000000..a1e90d0497a --- /dev/null +++ b/pkg/integration/tests/branch/merge_non_fast_forward.go @@ -0,0 +1,76 @@ +package branch + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MergeNonFastForward = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Merge a branch into another using non-fast-forward merge", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "alphabetical" + }, + SetupRepo: func(shell *Shell) { + shell.NewBranch("original-branch"). + EmptyCommit("one"). + NewBranch("branch1"). + EmptyCommit("branch1"). + Checkout("original-branch"). + NewBranchFrom("branch2", "original-branch"). + EmptyCommit("branch2"). + Checkout("original-branch") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("original-branch").IsSelected(), + Contains("branch1"), + Contains("branch2"), + ). + SelectNextItem(). + Press(keys.Branches.MergeIntoCurrentBranch) + + t.ExpectPopup().Menu(). + Title(Equals("Merge")). + TopLines( + Contains("Regular merge (fast-forward)"), + Contains("Regular merge (with merge commit)"), + ). + Select(Contains("Regular merge (with merge commit)")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("⏣─╮ Merge branch 'branch1' into original-branch").IsSelected(), + Contains("│ ◯ * branch1"), + Contains("◯─╯ one"), + ) + + // Check that branch2 shows the non-fast-forward option first + t.Views().Branches(). + Focus(). + NavigateToLine(Contains("branch2")). + Press(keys.Branches.MergeIntoCurrentBranch) + + t.ExpectPopup().Menu(). + Title(Equals("Merge")). + TopLines( + Contains("Regular merge (with merge commit)"), + Contains("Regular merge (fast-forward)"), + ). + Select(Contains("Regular merge (with merge commit)")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("⏣─╮ Merge branch 'branch2' into original-branch").IsSelected(), + Contains("│ ◯ * branch2"), + Contains("⏣─│─╮ Merge branch 'branch1' into original-branch"), + Contains("│ │ ◯ * branch1"), + Contains("◯─┴─╯ one"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index c08d68a1376..da21d1f0d3c 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -48,6 +48,8 @@ var tests = []*components.IntegrationTest{ branch.DeleteRemoteBranchWithDifferentName, branch.DeleteWhileFiltering, branch.DetachedHead, + branch.MergeFastForward, + branch.MergeNonFastForward, branch.MoveCommitsToNewBranchFromBaseBranch, branch.MoveCommitsToNewBranchFromMainBranch, branch.MoveCommitsToNewBranchKeepStacked, diff --git a/pkg/integration/tests/ui/mode_specific_keybinding_suggestions.go b/pkg/integration/tests/ui/mode_specific_keybinding_suggestions.go index 5cd412146f2..d64a22a38cc 100644 --- a/pkg/integration/tests/ui/mode_specific_keybinding_suggestions.go +++ b/pkg/integration/tests/ui/mode_specific_keybinding_suggestions.go @@ -103,7 +103,7 @@ var ModeSpecificKeybindingSuggestions = NewIntegrationTest(NewIntegrationTestArg Tap(func() { t.ExpectPopup().Menu(). Title(Equals("Merge")). - Select(Contains("Regular merge")). + Select(Contains("Regular merge (with merge commit)")). Confirm() t.Common().AcknowledgeConflicts()