Skip to content

Commit 6782f04

Browse files
authored
Add no-ff merge option (#4966)
This will put whatever git's default merge variant is as the first menu item, and add a second item which is the opposite (no-ff if the default is ff, and vice versa). Which one is the default depends on whether a fast-forward merge is possible, and whether users have set git's `merge.ff` config or lazygit's `git.merging.args` config. If users prefer to always have the same option first no matter whether it's applicable, they can make ff always appear first by setting git's `merge.ff` config to "true" or "only", or by setting lazygit's `git.merging.args` config to "--ff" or "--ff-only"; if they want no-ff to appear first, they can do that by setting git's `merge.ff` config to "false", or by setting lazygit's `git.merging.args` config to "--no-ff". Which of these they choose depends on whether they want the config to also apply to other git clients including the cli, or only to lazygit. Closes #4091.
2 parents d334bae + 6285402 commit 6782f04

File tree

9 files changed

+336
-51
lines changed

9 files changed

+336
-51
lines changed

pkg/commands/git_commands/branch.go

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -250,26 +250,51 @@ func (self *BranchCommands) Rename(oldName string, newName string) error {
250250
return self.cmd.New(cmdArgs).Run()
251251
}
252252

253-
type MergeOpts struct {
254-
FastForwardOnly bool
255-
Squash bool
256-
}
253+
type MergeVariant int
254+
255+
const (
256+
MERGE_VARIANT_REGULAR MergeVariant = iota
257+
MERGE_VARIANT_FAST_FORWARD
258+
MERGE_VARIANT_NON_FAST_FORWARD
259+
MERGE_VARIANT_SQUASH
260+
)
261+
262+
func (self *BranchCommands) Merge(branchName string, variant MergeVariant) error {
263+
extraArgs := func() []string {
264+
switch variant {
265+
case MERGE_VARIANT_REGULAR:
266+
return []string{}
267+
case MERGE_VARIANT_FAST_FORWARD:
268+
return []string{"--ff"}
269+
case MERGE_VARIANT_NON_FAST_FORWARD:
270+
return []string{"--no-ff"}
271+
case MERGE_VARIANT_SQUASH:
272+
return []string{"--squash", "--ff"}
273+
}
274+
275+
panic("shouldn't get here")
276+
}()
257277

258-
func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
259-
if opts.Squash && opts.FastForwardOnly {
260-
panic("Squash and FastForwardOnly can't both be true")
261-
}
262278
cmdArgs := NewGitCmd("merge").
263279
Arg("--no-edit").
264280
Arg(strings.Fields(self.UserConfig().Git.Merging.Args)...).
265-
ArgIf(opts.FastForwardOnly, "--ff-only").
266-
ArgIf(opts.Squash, "--squash", "--ff").
281+
Arg(extraArgs...).
267282
Arg(branchName).
268283
ToArgv()
269284

270285
return self.cmd.New(cmdArgs).Run()
271286
}
272287

288+
// Returns whether refName can be fast-forward merged into the current branch
289+
func (self *BranchCommands) CanDoFastForwardMerge(refName string) bool {
290+
cmdArgs := NewGitCmd("merge-base").
291+
Arg("--is-ancestor").
292+
Arg("HEAD", refName).
293+
ToArgv()
294+
err := self.cmd.New(cmdArgs).DontLog().Run()
295+
return err == nil
296+
}
297+
273298
// Only choose between non-empty, non-identical commands
274299
func (self *BranchCommands) allBranchesLogCandidates() []string {
275300
return lo.Uniq(lo.WithoutEmpty(self.UserConfig().Git.AllBranchesLogCmds))

pkg/commands/git_commands/branch_test.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ func TestBranchMerge(t *testing.T) {
122122
scenarios := []struct {
123123
testName string
124124
userConfig *config.UserConfig
125-
opts MergeOpts
125+
variant MergeVariant
126126
branchName string
127127
expected []string
128128
}{
129129
{
130130
testName: "basic",
131131
userConfig: &config.UserConfig{},
132-
opts: MergeOpts{},
132+
variant: MERGE_VARIANT_REGULAR,
133133
branchName: "mybranch",
134134
expected: []string{"merge", "--no-edit", "mybranch"},
135135
},
@@ -142,7 +142,7 @@ func TestBranchMerge(t *testing.T) {
142142
},
143143
},
144144
},
145-
opts: MergeOpts{},
145+
variant: MERGE_VARIANT_REGULAR,
146146
branchName: "mybranch",
147147
expected: []string{"merge", "--no-edit", "--merging-args", "mybranch"},
148148
},
@@ -155,16 +155,30 @@ func TestBranchMerge(t *testing.T) {
155155
},
156156
},
157157
},
158-
opts: MergeOpts{},
158+
variant: MERGE_VARIANT_REGULAR,
159159
branchName: "mybranch",
160160
expected: []string{"merge", "--no-edit", "--arg1", "--arg2", "mybranch"},
161161
},
162162
{
163-
testName: "fast forward only",
163+
testName: "fast-forward merge",
164164
userConfig: &config.UserConfig{},
165-
opts: MergeOpts{FastForwardOnly: true},
165+
variant: MERGE_VARIANT_FAST_FORWARD,
166166
branchName: "mybranch",
167-
expected: []string{"merge", "--no-edit", "--ff-only", "mybranch"},
167+
expected: []string{"merge", "--no-edit", "--ff", "mybranch"},
168+
},
169+
{
170+
testName: "non-fast-forward merge",
171+
userConfig: &config.UserConfig{},
172+
variant: MERGE_VARIANT_NON_FAST_FORWARD,
173+
branchName: "mybranch",
174+
expected: []string{"merge", "--no-edit", "--no-ff", "mybranch"},
175+
},
176+
{
177+
testName: "squash merge",
178+
userConfig: &config.UserConfig{},
179+
variant: MERGE_VARIANT_SQUASH,
180+
branchName: "mybranch",
181+
expected: []string{"merge", "--no-edit", "--squash", "--ff", "mybranch"},
168182
},
169183
}
170184

@@ -174,7 +188,7 @@ func TestBranchMerge(t *testing.T) {
174188
ExpectGitArgs(s.expected, "", nil)
175189
instance := buildBranchCommands(commonDeps{runner: runner, userConfig: s.userConfig})
176190

177-
assert.NoError(t, instance.Merge(s.branchName, s.opts))
191+
assert.NoError(t, instance.Merge(s.branchName, s.variant))
178192
runner.CheckForMissingCalls()
179193
})
180194
}

pkg/commands/git_commands/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ func (self *ConfigCommands) GetRebaseUpdateRefs() bool {
9797
return self.gitConfig.GetBool("rebase.updateRefs")
9898
}
9999

100+
func (self *ConfigCommands) GetMergeFF() string {
101+
return self.gitConfig.Get("merge.ff")
102+
}
103+
100104
func (self *ConfigCommands) DropConfigCache() {
101105
self.gitConfig.DropCache()
102106
}

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

Lines changed: 110 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -381,38 +381,103 @@ func (self *MergeAndRebaseHelper) MergeRefIntoCheckedOutBranch(refName string) e
381381
return errors.New(self.c.Tr.CantMergeBranchIntoItself)
382382
}
383383

384+
wantFastForward, wantNonFastForward := self.fastForwardMergeUserPreference()
385+
canFastForward := self.c.Git().Branch.CanDoFastForwardMerge(refName)
386+
387+
var firstRegularMergeItem *types.MenuItem
388+
var secondRegularMergeItem *types.MenuItem
389+
var fastForwardMergeItem *types.MenuItem
390+
391+
if !wantNonFastForward && (wantFastForward || canFastForward) {
392+
firstRegularMergeItem = &types.MenuItem{
393+
Label: self.c.Tr.RegularMergeFastForward,
394+
OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_REGULAR),
395+
Key: 'm',
396+
Tooltip: utils.ResolvePlaceholderString(
397+
self.c.Tr.RegularMergeFastForwardTooltip,
398+
map[string]string{
399+
"checkedOutBranch": checkedOutBranchName,
400+
"selectedBranch": refName,
401+
},
402+
),
403+
}
404+
fastForwardMergeItem = firstRegularMergeItem
405+
406+
secondRegularMergeItem = &types.MenuItem{
407+
Label: self.c.Tr.RegularMergeNonFastForward,
408+
OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_NON_FAST_FORWARD),
409+
Key: 'n',
410+
Tooltip: utils.ResolvePlaceholderString(
411+
self.c.Tr.RegularMergeNonFastForwardTooltip,
412+
map[string]string{
413+
"checkedOutBranch": checkedOutBranchName,
414+
"selectedBranch": refName,
415+
},
416+
),
417+
}
418+
} else {
419+
firstRegularMergeItem = &types.MenuItem{
420+
Label: self.c.Tr.RegularMergeNonFastForward,
421+
OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_REGULAR),
422+
Key: 'm',
423+
Tooltip: utils.ResolvePlaceholderString(
424+
self.c.Tr.RegularMergeNonFastForwardTooltip,
425+
map[string]string{
426+
"checkedOutBranch": checkedOutBranchName,
427+
"selectedBranch": refName,
428+
},
429+
),
430+
}
431+
432+
secondRegularMergeItem = &types.MenuItem{
433+
Label: self.c.Tr.RegularMergeFastForward,
434+
OnPress: self.RegularMerge(refName, git_commands.MERGE_VARIANT_FAST_FORWARD),
435+
Key: 'f',
436+
Tooltip: utils.ResolvePlaceholderString(
437+
self.c.Tr.RegularMergeFastForwardTooltip,
438+
map[string]string{
439+
"checkedOutBranch": checkedOutBranchName,
440+
"selectedBranch": refName,
441+
},
442+
),
443+
}
444+
fastForwardMergeItem = secondRegularMergeItem
445+
}
446+
447+
if !canFastForward {
448+
fastForwardMergeItem.DisabledReason = &types.DisabledReason{
449+
Text: utils.ResolvePlaceholderString(
450+
self.c.Tr.CannotFastForwardMerge,
451+
map[string]string{
452+
"checkedOutBranch": checkedOutBranchName,
453+
"selectedBranch": refName,
454+
},
455+
),
456+
}
457+
}
458+
384459
return self.c.Menu(types.CreateMenuOptions{
385460
Title: self.c.Tr.Merge,
386461
Items: []*types.MenuItem{
462+
firstRegularMergeItem,
463+
secondRegularMergeItem,
387464
{
388-
Label: self.c.Tr.RegularMerge,
389-
OnPress: self.RegularMerge(refName),
390-
Key: 'm',
391-
Tooltip: utils.ResolvePlaceholderString(
392-
self.c.Tr.RegularMergeTooltip,
393-
map[string]string{
394-
"checkedOutBranch": checkedOutBranchName,
395-
"selectedBranch": refName,
396-
},
397-
),
398-
},
399-
{
400-
Label: self.c.Tr.SquashMergeUncommittedTitle,
465+
Label: self.c.Tr.SquashMergeUncommitted,
401466
OnPress: self.SquashMergeUncommitted(refName),
402467
Key: 's',
403468
Tooltip: utils.ResolvePlaceholderString(
404-
self.c.Tr.SquashMergeUncommitted,
469+
self.c.Tr.SquashMergeUncommittedTooltip,
405470
map[string]string{
406471
"selectedBranch": refName,
407472
},
408473
),
409474
},
410475
{
411-
Label: self.c.Tr.SquashMergeCommittedTitle,
476+
Label: self.c.Tr.SquashMergeCommitted,
412477
OnPress: self.SquashMergeCommitted(refName, checkedOutBranchName),
413478
Key: 'S',
414479
Tooltip: utils.ResolvePlaceholderString(
415-
self.c.Tr.SquashMergeCommitted,
480+
self.c.Tr.SquashMergeCommittedTooltip,
416481
map[string]string{
417482
"checkedOutBranch": checkedOutBranchName,
418483
"selectedBranch": refName,
@@ -423,26 +488,26 @@ func (self *MergeAndRebaseHelper) MergeRefIntoCheckedOutBranch(refName string) e
423488
})
424489
}
425490

426-
func (self *MergeAndRebaseHelper) RegularMerge(refName string) func() error {
491+
func (self *MergeAndRebaseHelper) RegularMerge(refName string, variant git_commands.MergeVariant) func() error {
427492
return func() error {
428493
self.c.LogAction(self.c.Tr.Actions.Merge)
429-
err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{})
494+
err := self.c.Git().Branch.Merge(refName, variant)
430495
return self.CheckMergeOrRebase(err)
431496
}
432497
}
433498

434499
func (self *MergeAndRebaseHelper) SquashMergeUncommitted(refName string) func() error {
435500
return func() error {
436501
self.c.LogAction(self.c.Tr.Actions.SquashMerge)
437-
err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{Squash: true})
502+
err := self.c.Git().Branch.Merge(refName, git_commands.MERGE_VARIANT_SQUASH)
438503
return self.CheckMergeOrRebase(err)
439504
}
440505
}
441506

442507
func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranchName string) func() error {
443508
return func() error {
444509
self.c.LogAction(self.c.Tr.Actions.SquashMerge)
445-
err := self.c.Git().Branch.Merge(refName, git_commands.MergeOpts{Squash: true})
510+
err := self.c.Git().Branch.Merge(refName, git_commands.MERGE_VARIANT_SQUASH)
446511
if err = self.CheckMergeOrRebase(err); err != nil {
447512
return err
448513
}
@@ -459,6 +524,31 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch
459524
}
460525
}
461526

527+
// Returns wantsFastForward, wantsNonFastForward. These will never both be true, but they can both be false.
528+
func (self *MergeAndRebaseHelper) fastForwardMergeUserPreference() (bool, bool) {
529+
// Check user config first, because it takes precedence over git config
530+
mergingArgs := self.c.UserConfig().Git.Merging.Args
531+
if strings.Contains(mergingArgs, "--ff") { // also covers "--ff-only"
532+
return true, false
533+
}
534+
535+
if strings.Contains(mergingArgs, "--no-ff") {
536+
return false, true
537+
}
538+
539+
// Then check git config
540+
mergeFfConfig := self.c.Git().Config.GetMergeFF()
541+
if mergeFfConfig == "true" || mergeFfConfig == "only" {
542+
return true, false
543+
}
544+
545+
if mergeFfConfig == "false" {
546+
return false, true
547+
}
548+
549+
return false, false
550+
}
551+
462552
func (self *MergeAndRebaseHelper) ResetMarkedBaseCommit() error {
463553
self.c.Modes().MarkedBaseCommit.Reset()
464554
self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)

pkg/i18n/english.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@ type TranslationSet struct {
2323
StagedChanges string
2424
StagingTitle string
2525
MergingTitle string
26-
SquashMergeUncommittedTitle string
27-
SquashMergeCommittedTitle string
28-
SquashMergeUncommitted string
29-
SquashMergeCommitted string
30-
RegularMergeTooltip string
3126
NormalTitle string
3227
LogTitle string
3328
LogXOfYTitle string
@@ -268,8 +263,16 @@ type TranslationSet struct {
268263
RefreshFiles string
269264
FocusMainView string
270265
Merge string
271-
RegularMerge string
272266
MergeBranchTooltip string
267+
RegularMergeFastForward string
268+
RegularMergeFastForwardTooltip string
269+
CannotFastForwardMerge string
270+
RegularMergeNonFastForward string
271+
RegularMergeNonFastForwardTooltip string
272+
SquashMergeUncommitted string
273+
SquashMergeUncommittedTooltip string
274+
SquashMergeCommitted string
275+
SquashMergeCommittedTooltip string
273276
ConfirmQuit string
274277
SwitchRepo string
275278
AllBranchesLogGraph string
@@ -1108,8 +1111,6 @@ func EnglishTranslationSet() *TranslationSet {
11081111
EasterEgg: "Easter egg",
11091112
UnstagedChanges: "Unstaged changes",
11101113
StagedChanges: "Staged changes",
1111-
SquashMergeUncommittedTitle: "Squash merge and leave uncommitted",
1112-
SquashMergeCommittedTitle: "Squash merge and commit",
11131114
StagingTitle: "Main panel (staging)",
11141115
MergingTitle: "Main panel (merging)",
11151116
NormalTitle: "Main panel (normal)",
@@ -1352,8 +1353,16 @@ func EnglishTranslationSet() *TranslationSet {
13521353
RefreshFiles: `Refresh files`,
13531354
FocusMainView: "Focus main view",
13541355
Merge: `Merge`,
1355-
RegularMerge: "Regular merge",
13561356
MergeBranchTooltip: "View options for merging the selected item into the current branch (regular merge, squash merge)",
1357+
RegularMergeFastForward: "Regular merge (fast-forward)",
1358+
RegularMergeFastForwardTooltip: "Fast-forward '{{.checkedOutBranch}}' to '{{.selectedBranch}}' without creating a merge commit.",
1359+
CannotFastForwardMerge: "Cannot fast-forward '{{.checkedOutBranch}}' to '{{.selectedBranch}}'",
1360+
RegularMergeNonFastForward: "Regular merge (with merge commit)",
1361+
RegularMergeNonFastForwardTooltip: "Merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}', creating a merge commit.",
1362+
SquashMergeUncommitted: "Squash merge and leave uncommitted",
1363+
SquashMergeUncommittedTooltip: "Squash merge '{{.selectedBranch}}' into the working tree.",
1364+
SquashMergeCommitted: "Squash merge and commit",
1365+
SquashMergeCommittedTooltip: "Squash merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}' as a single commit.",
13571366
ConfirmQuit: `Are you sure you want to quit?`,
13581367
SwitchRepo: `Switch to a recent repo`,
13591368
AllBranchesLogGraph: `Show/cycle all branch logs`,
@@ -1438,9 +1447,6 @@ func EnglishTranslationSet() *TranslationSet {
14381447
InteractiveRebaseTooltip: "Begin an interactive rebase with a break at the start, so you can update the TODO commits before continuing.",
14391448
RebaseOntoBaseBranchTooltip: "Rebase the checked out branch onto its base branch (i.e. the closest main branch).",
14401449
MustSelectTodoCommits: "When rebasing, this action only works on a selection of TODO commits.",
1441-
SquashMergeUncommitted: "Squash merge '{{.selectedBranch}}' into the working tree.",
1442-
SquashMergeCommitted: "Squash merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}' as a single commit.",
1443-
RegularMergeTooltip: "Merge '{{.selectedBranch}}' into '{{.checkedOutBranch}}'.",
14441450
FwdNoUpstream: "Cannot fast-forward a branch with no upstream",
14451451
FwdNoLocalUpstream: "Cannot fast-forward a branch whose remote is not registered locally",
14461452
FwdCommitsToPush: "Cannot fast-forward a branch with commits to push",

0 commit comments

Comments
 (0)