lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Automatic staging of files during a merge conflict / interactive rebase

Open randoragon opened this issue 2 years ago • 6 comments

Technically this is not a bug, but an inconvenient behavior that should be possible to disable, in my opinion.

During a merge or interactive rebase, when conflicts are encountered, lazygit attempts to resolve them automatically by staging all files, and, if it's possible to proceed, prompts the user with "All merge conflicts resolved. Continue?". I find this behavior highly disruptive, because there are plenty of scenarios in which resolving a merge conflict is not as simple as git add ., even if there are no conflict markers present in files. In general, this makes me not want to rely on lazygit during some of these merges, because I'm afraid of it ruining my staging area while I want to carefully review the unstaged changes and stage the relevant bits little by little manually.

To Reproduce

  1. Cause a merge conflict that can be resolved with git add ..
  2. Open lazygit.
  3. Within a few seconds, lazygit stages all files and prompts the user with "All merge conflicts resolved. Continue?".

Expected behavior

Lazygit should not stage any files and should not prompt the user in any way, giving them manual control over the rebase. I would like to see a config option to turn off this behavior entirely.

Version info: commit=v0.40.2, build date=2023-08-12T17:47:33Z, build source=binaryRelease, version=0.40.2, os=linux, arch=amd64, git version=2.42.1
git version 2.42.1

Additional context I run into this problem the most frequently when I'm working on projects with git submodules. What tends to happen:

  1. I have 2 matching feature branches in both the parent repo and its submodule. They are being developed in parallel, i.e. I may have commits [A, B, C] on the submodule feature branch, and corresponding commits [A, B, C] on the parent repo feature branch, which bump the submodule.
  2. I want to rebase both repos' feature branches on top of their masters. The way to do this in git is to first rebase the submodule branch, and then rebase the parent repo branch. But during the parent repo rebase, it is necessary to stop at each of the A, B and C commits, go to the submodule, check out the correct commit, go back to the main repo, git add, and only then the conflict is properly resolved. git add . is a legal way to resolve the conflict, but it leads to "squashing" all [A, B, C] commits of the submodule repo into a single commit on the main repo, which is undesired.

randoragon avatar Nov 08 '23 11:11 randoragon

[side note/rant]: Submodules suck. They may be ok for pulling in third-party dependencies that only change every so often, but for stuff that people are actively developing, they are a PITA. I'm so glad that we switched to a monorepo at work.

On to the real issue: yes, I can see how lazygit's behavior can be annoying, and I agree it would be good to have an option to disable it. Personally I happen to like it, most of the time, so I would keep it enabled. In fact, I often have the opposite problem, which is that lazygit doesn't stage changes aggressively enough. The use case is that you fix the conflicts in your editor (I rarely use lazygit's merge editor for that); once you saved the last one, lazygit stages things and brings up the "Continue?" prompt, but in the background where you don't see it. Meanwhile, I find that things don't compile yet, so I continue to fix some compiler errors; now, when I return to lazygit, answering "yes" to the continue prompt doesn't work because not all files are staged.

stefanhaller avatar Nov 08 '23 12:11 stefanhaller

Personally I happen to like it, most of the time, so I would keep it enabled. In fact, I often have the opposite problem, which is that lazygit doesn't stage changes aggressively enough.

That's a good counterexample, I have that happen to me sometimes as well. Part of the reason I dislike the "Continue?" prompt is because it's not always clear what has been staged and what hasn't. Generally in such cases I'd rather be forced to double-check than trust a tool that only does what I want 99% of the time (especially given how annoying correcting a rebase/merge is in git).

I totally agree that the current behavior should be opt-out, not opt-in. New users might get surprised by it, but find it useful, or disable it if they don't. On the contrary, if it was opt-out, it could go over many potential users's heads, since it's not an obvious feature.

randoragon avatar Nov 08 '23 14:11 randoragon

I couldn't agree more

BrdgYin avatar Nov 09 '23 10:11 BrdgYin

I've run into this problem often enough now when doing lazygit interactive rebase + VSCodium merge conflict editor. I'm not using submodules at all, so the problem exists without them. Also, it occurs when I'm not using lazygits merge editor. It just randomly happens in the background while I'm trying to resolve conflicts in VSCodium. I had to abort some big rebases because I couldn't salvage my worktree anymore. An option to disable this behaviour would be greatly appreciated! (IMHO it should be the default, then maybe add a tooltip to raise awareness for the option)

turion avatar Apr 22 '24 13:04 turion

This caught me off guard where it was happening in the background and I did not know what happened to the files in conflict. With Git LFS enabled, merge conflict resolution by Lazygit is plain wrong with this behavior. Unreal is capable of handling binary merge conflicts of its assets, but with Lazygit running, I cannot resolve such conflicts.

I would prefer this behaviour to be OFF by default, but what matters more is that it is driven by a config.

samaursa avatar Aug 28 '24 19:08 samaursa

Here's a PR: #3870.

Could you please test it and see if this addresses the problem for you?

stefanhaller avatar Aug 28 '24 21:08 stefanhaller

Meanwhile, I find that things don't compile yet, so I continue to fix some compiler errors; now, when I return to lazygit, answering "yes" to the continue prompt doesn't work because not all files are staged.

For the record this annoys me too: I'd like it to prompt me to stage all and continue

jesseduffield avatar Aug 31 '24 00:08 jesseduffield

@stefanhaller I'd be happy to test it, however I have been relying on the binary builds. Are there a set of binary builds of the PR? If not, I'll try to build/run it from repo following the contribution guide.

samaursa avatar Aug 31 '24 02:08 samaursa

Meanwhile, I find that things don't compile yet, so I continue to fix some compiler errors; now, when I return to lazygit, answering "yes" to the continue prompt doesn't work because not all files are staged.

For the record this annoys me too: I'd like it to prompt me to stage all and continue

Are you thinking about changing the confirmation to a different one while it is showing if it notices that there are unstaged changes now? I'd have to look into how we might do this, but I'm also worried that the change might be too easy to miss. Or do you mean an additional prompt after you confirmed the first one? That would be much easier.

stefanhaller avatar Aug 31 '24 06:08 stefanhaller

@stefanhaller I'd be happy to test it, however I have been relying on the binary builds. Are there a set of binary builds of the PR? If not, I'll try to build/run it from repo following the contribution guide.

We don't have builds for PRs, sorry. But lazygit is very easy to build from source, all you need is install go for your platform. It has no other dependencies.

I'm very interested in your feedback on this before I merge. The other people who requested this too! @randoragon @turion @BrdgYin

stefanhaller avatar Aug 31 '24 06:08 stefanhaller

Are you thinking about changing the confirmation to a different one while it is showing if it notices that there are unstaged changes now? I'd have to look into how we might do this, but I'm also worried that the change might be too easy to miss. Or do you mean an additional prompt after you confirmed the first one? That would be much easier.

Assuming you mean the latter, I opened a PR to do that: #3879

stefanhaller avatar Aug 31 '24 15:08 stefanhaller

I'm very interested in your feedback on this before I merge. The other people who requested this too! @randoragon @turion @BrdgYin

Thanks for your hard work, I should be able to check today or tomorrow, hope that's okay.

randoragon avatar Aug 31 '24 16:08 randoragon

I cloned and built the https://github.com/jesseduffield/lazygit/pull/3870/commits/93f2961e4db74d266b5547a68e3a63d9c68fe99f commit, placed

git:
  autoStageResolvedConflicts: false

in ~/.config/lazygit/config.ymland ran a very simple experiment on a fresh git repo:

  1. Init commit a file with any contents.
  2. Create a feature branch.
  3. Commit a modification to the file on the feature branch, and a different modification on the main branch.
  4. Merge/rebase one branch onto the other (tried both) to cause a conflict.
  5. Open lazygit, edit the conflicting file with an external editor and get rid of the conflict markers.
  6. After the above step, upstream lazygit auto-stages the file and asks "All merge conflicts resolved. Continue?". PR lazygit does not do that, the edited file remains unstaged and no prompts show up.

Seems good, this is exactly the behavior I was hoping for @stefanhaller!

randoragon avatar Sep 01 '24 11:09 randoragon

@randoragon Thanks for testing. I'll consider this enough confirmation, and am going to merge.

stefanhaller avatar Sep 02 '24 16:09 stefanhaller

Was able to confirm that the above fix works as intended. LFS file conflicts are no longer auto-merged.

samaursa avatar Sep 03 '24 20:09 samaursa

@samaursa Thanks for testing!

stefanhaller avatar Sep 04 '24 05:09 stefanhaller