Can we have a no-ff (non-fast forward) merge? Revisit #785

[doublethefish]

Is your feature request related to a problem? Please describe.
Depending on which project I am contributing to, I often switch between using --no-ff or not. Often it is older projects that prefer --no-ff merge strategies. An inbuilt --no-ff optional merge type in lazygit is needed, in addition to the config and custom commands.

Describe the solution you'd like
It would be incredibly useful to have a default merge type that adheres to the config as per solution to #785, AND an explicit --no-ff.

Describe alternatives you've considered

1. global config ~/.config/.*/config.yml
   • switching between projects that require --no-ff requires that I drop to terminal
   • or configure each repo separately (see below)
   • requires that global config is sync'd between system (which I have so maybe not an issue for me)
2. silent per repo config <repo>/.git/lazygit.yml
   • If I were to add <repo>/.git/lazygit.yml to each repo I need to ensure that on each checkout of that repo, on each machine, that I have the config.
3. committed per repo config <repo>/**/.lazygit.yml
   • requires committing to each repo (overhead)
   • pollutes each repo
   • is reliant on maintainers being ok with committed config for tools they may not use/support
4. custom commands
   • similar to 1 and 2 above

Additional context
--no-ff is such a typical, and useful, workflow but is no longer the standard so having a one-size-fits all solution in this case doesn't work. Two merge types are needed.

However in the keyboard-wizard world of lazygit I'm not quite sure how this would be best implmented.

[oxcrow]

I support this enhancement request.

Even within the same git repo it is beneficial to use the fast forward and no fast forward methods of merging to ensure we retain a detailed history of our commits.

For example on development branch I want to merge using fast forward.

However on master branch I would want to use no fast forward merge so that the features/fixes from the development branch are grouped nicely in history and can help in understanding the project's development.

This feature is needed.

[stefanhaller]

I'm not convinced we should add this to lazygit; my suspicion is that only few people need such an option. This is pure speculation of course, as long as we don't conduct proper user research. And I'm happy to be proven wrong if enough people upvote this.

It would be easy to add another menu entry to the merge menu for this, but then, some people might want --no-ff whereas others might want -ff-only. So do we need to add two menu entries? This would feel like undue noise in the menu to me.

This is easily solved by using a custom command, which to me feels like an adequate solution to the problem:

customCommands:
  - key: <whatever>
    command: git merge --ff-only {{.SelectedLocalBranch.Name}}
    context: localBranches
    description: Merge using --ff-only

[oxcrow]

@stefanhaller thank you for providing an alternative solution.

Regarding if "only few people need such an option" I would say that is correct.

However that is because most people use git often only follow the best practices that they have grown accustomed to, and over time these best practices change. So it is beneficial to have such features available if some amount of users are asking for it (like me and others) because we consider it to be a good practice instead of the default fast-forward merge.

So features like this only become best practice when they are available to the users.

If lazygit does not have this feature most users won't know about it and won't use it.

I think this feature should be implemented.

[doublethefish]

@stefanhaller Thanks for the interesting input, I appreciate the chance to debate this with you.

So do we need to add two menu entries?

No, we just need to add one. The three existing options remains.

This would feel like undue noise in the menu to me.

Assuming that you really do feel that a single new option on the Merge sub-menu would be undue noise, let us look at it, see if we can make it feel a bit more reasonable/objective/measurable.

Cur:

vs:

To me this second option does not feel like undue noise, especially if you compare it to the Rebase menu which is significantly visually noisier than the merge menu; it is visually noisier because of the greater number of characters:

Putting the noise complaint to one side for a second, and just focusing on the utility of git, it is completely reasonable to expect 1st order, core features of git to be available. As you can see the other merge-specific feature, --squash plus the utility squash+inspect variant, is already there. Unrelated histories doesn't make sense in lazygit. Outside of merge, almost all of the other core git features are there; I think --no-ff and rebase's --exec are the only two missing.

This is easily solved by using a custom command, which to me feels like an adequate solution to the problem:

This was covered in the OP. It assumes (presumes) how different teams and projects work and use core features. That does not make sense to me in this instance.

[stefanhaller]

I have absolutely no problem with adding a single additional entry to the menu; however, I don't think this addresses the problem enough. Some people may want to use --ff-only, so those will complain that their favorite option is still not supported. Also, many people will probably have --no-ff in their git.merging.args config (see #797), so for those a --no-ff menu entry doesn't change anything, but they might want to have a --ff (or --ff-only) option to override their config.

What I could imagine is having a menu such as this:

╭─Merge───────────────────────────────────
│m Regular merge
│f Merge with fast-forward option...
│s Squash merge and leave uncommitted
│S Squash merge and commit
│  Cancel
╰─────────────────────────────────────────

Choosing the second entry will bring up another menu:

╭─Merge with fast-forward option──────────
│--no-ff
│--ff
│--ff-only
╰─────────────────────────────────────────

Probably with tooltips explaining what each of these mean, for people who are new to git.

One minor downside is that people who use git.merging.args: --no-ff and then pick the --ff choice will see git merge --no-ff --ff branch-name in the log output. This will work correctly (I think), just look slightly confusing.

What do you think?

[doublethefish]

re: git.merging.args: --no-ff very good point, I had not considered that.

Then the options would be:

╭─Merge with fast-forward option──────────
│ m  Regular merge 
| s  Squash merge and leave uncommitted
| S  Squash merge and commit
│ f  Fast-forward merge (linear history)
│ n  No fast-forward merge (merge commit)
│ 0  Fast-forward only (refuse if fast-forward not possible)
|    Cancel
╰─────────────────────────────────────────

I feel like those would be reasonable and useful, esp as we would want to allow git.merging.args: --no-ff which would impact the "m Regular merge". The reason for not needing to duplicate the squash options per ff option is that --no-squash is implied in the common use cases with ff.

Another reason for not duplicating the Squash options is that - whilst I would be curious how many people use the Squash options - my experience, in every git using project I have seen, is that the Squashing is done whilst prepping a PR, or on the PR tooling, or by the dev-ops infra. That said, because they are in lazygit, I have actually used both of them myself once or twice.

p.s. I absolutely love lazygit. The rebasing tools save me so much time/effort.

[stefanhaller]

The reason for not needing to duplicate the squash options per ff option is that --no-squash is implied in the common use cases with ff.

I'd say it's the other way round: when squash merging, the concepts of fast-forwarding or not are meaningless. We are probably saying the same though.

You didn't react to my suggestion of putting the three additional merge options in a submenu, any thoughts about that? I still prefer this over "polluting" the main merge menu with them.

Another reason for not duplicating the Squash options is that - whilst I would be curious how many people use the Squash options - my experience, in every git using project I have seen, is that the Squashing is done whilst prepping a PR, or on the PR tooling, or by the dev-ops infra. That said, because they are in lazygit, I have actually used both of them myself once or twice.

I use squash merging very frequently locally, when creating a local integration branch that assembles a bunch of PRs that I want to test together. I find squash merging to be a very convenient workflow for this: if one of those branches changes, I just drop the previous merge commit for that branch and squash merge it again.

p.s. I absolutely love lazygit. The rebasing tools save me so much time/effort.

Same here. 😉

[doublethefish]

You didn't react to my suggestion of putting the three additional merge options in a submenu, any thoughts about that? I still prefer this over "polluting" the main merge menu with them.

I see, I had missed the significance of that bit, my bad.

Do you mean:

╭─Merge─────────────────────────────────────
│ m  Regular merge 
| s  Squash merge and leave uncommitted
| S  Squash merge and commit
| o  Merge options
|    Cancel
╰─────────────────────────────────────────
  ||
 \  /

╭─Merge options─────────────────────────────────
│ m  Default merge (set in git configs)
│ f  Fast-forward merge (linear history)
│ s  Fast-forward merge with squash (linear history)
│ n  Non fast-forward merge (merge commit)
│ S  Non fast-forward merge with squash (merge commit)
│ F  Fast-forward only (refuse if fast-forward not possible)
|    Cancel
╰─────────────────────────────────────────

Yeah, that seem reasonable. Probably only if we need to duplicate the squash options as above, otherwise it could fit into the main merge menu.

I guess that the keyboard-shortcut that would be something like m Merge -> o Merge options -> n No fast forward merge instead of m Merge -> n No fast forward merge, and that's ok with me - the keyboard shortcuts to the submenu would make it viable.

I'm happy either way.

The reason for not needing to duplicate the squash options per ff option is that --no-squash is implied in the common use cases with ff.

I'd say it's the other way round: when squash merging, the concepts of fast-forwarding or not are meaningless. We are probably saying the same though.

You are right, the concepts of FF + squash are not meaningless - I think in my comment I was in my idealist-mindset. That is, in linear-history projects that use squashing, you don't need (want?) additional merge commits, so ff is implied - in less ideal, and often much messier projects, that do not enforce linear-history, you do often see squash-and-merge, so non-ff is implied there. My opinion on the latter is that it's bad project hygiene because, whilst it does reduces merge spaghetti, it still introduces some - but then high-velocity teams are going to high-velocity!

[stefanhaller]

Do you mean:

No, not quite. There's no need to guess what I mean, I made a very concrete suggestion in this comment above.

In your "Merge options" submenu, the m options is redundant (same as the m option in the main menu), and the squash options don't need to be repeated from the main menu either (and your S option doesn't make sense, see below). What's left is exactly the three ff-related options from my suggestion.

You are right, the concepts of FF + squash are not meaningless

Yes they are, and that's what I said. If you squash-merge, you create a non-merge commit; the question of fast-forward or not is meaningless in that case.

[stefanhaller]

I was about to start working on adding the proposed solution. After thinking about it a bit more, I changed my mind; adding an extra menu with three options doesn't make sense to me after all.

First of all, offering an --ff-only option doesn't make sense at all. This would try to do an ff merge if possible, and error out if not; but we can perfectly determine up front whether an ff merge is possible, and make this a regular --ff command which is simply disabled when not applicable.

Second, we can work out how the regular merge is going to behave, fast-forward or not (this depends on whether the branches diverged, the value of the merge.ff git config, and the value of the git.merge.args lazygit config). Then, all we need to do is add the opposite option in the main merge menu.

Which means that the menu has these possible states:

1. If typing git merge branch on the command line would do a ff merge, it looks like this:

╭─Merge───────────────────────────────────
│m Regular merge (fast-forward)
│n Regular merge (non-fast-forward)
│s Squash merge and leave uncommitted
│S Squash merge and commit
│  Cancel
╰─────────────────────────────────────────

2. If typing git merge branch on the command line would do a non-ff merge (either because the branches diverged, or because the merge.ff config is set to false), it looks like this:

╭─Merge───────────────────────────────────
│m Regular merge (non-fast-forward)
│f Regular merge (fast-forward)
│s Squash merge and leave uncommitted
│S Squash merge and commit
│  Cancel
╰─────────────────────────────────────────

In the case that the branches diverged, the "Regular merge (fast-forward)" item would be disabled (strike through).

One possible downside of this approach is that the position of the menu items change depending on circumstances, which is usually a bad thing for muscle memory; on the other hand, the default option is always first (i.e. M enter), which is a good thing. Not sure how to resolve this tension, but at least the keybindings for the second item are different.

I'm going to start implementing this tomorrow unless somebody has strong objections.

[stefanhaller]

Here's the PR that implements this: #4966

One possible downside of this approach is that the position of the menu items change depending on circumstances, which is usually a bad thing for muscle memory;

I addressed this in the PR by keeping the order of the menu items fixed if users set either git's merge.ff config or lazygit's git.merging.args config. See PR description for details.