Add an option to delete both remote and local branch in one menu item

[ekorchmar]

• PR Description

Adds menu option to delete the branch on remote first, then locally.

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go generate ./...)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[stefanhaller]

Sorry for taking so long to review this.

I'm afraid this very simple approach doesn't quite do it. What happens here is that you get the confirmation prompt for deleting the remote branch, and while the prompt is showing, it already goes ahead and deletes the local branch. If you answer no to the prompt, it has already deleted the local branch, which is not what I would expect.

Also, if the local branch is ahead of its remote branch, you would usually get the confirmation "The branch isn't fully merged, do you still want to delete if?". Except in this case you don't get that prompt, because the other prompt is still showing, and we can only have one popup open at a time. So it will just not delete the local branch in this case, but silently, without saying why. This is also not good enough.

I think we should restructure the code so that there is only one confirmation prompt asking you if you want to delete both branches, and when answering yes, it should do that in one WithWaitingStatus call. This needs a little bit of refactoring.

I'm a little unsure what to do about the "The branch isn't fully merged" situation. We could either handle this in the same way as we normally do, which means that you would get a second confirmation popup in that case. It also means that we need to delete the local branch first, otherwise you would always get that warning. Alternatively, we could consider the first popup confirmation enough that they really want to delete those branches, and just use -D for the local branch delete right away.

I'm unsure here and would like to hear @jesseduffield's opinion on that last question.

[jesseduffield]

I agree in general, except for the part about skipping the warning. I think we should show the warning after the user has confirmed the prompt.

It also means that we need to delete the local branch first

I'm not following: I thought that that warning only shows up in relation to the local branch.

[stefanhaller]

I agree in general, except for the part about skipping the warning. I think we should show the warning after the user has confirmed the prompt.

That's fine with me, I did mention this as one of the options that I couldn't decide between.

It also means that we need to delete the local branch first

I'm not following: I thought that that warning only shows up in relation to the local branch.

No, it considers both the current HEAD and the branch's upstream.

Side note: I do find this warning annoying in many cases; I wish it would also consider my other local (or remote) branches, or at least main/master. I often get the warning when deleting a branch that I just merged on github, but before I checked out main and pulled it. But that's something to improve in git, not in lazygit.

Although, wait a moment: we could totally do the check on our side (in what we think is a better way than git does it), and always use -D. What do you think? (To be clear, this is unrelated to this PR, I'm talking about a general improvement to the "delete local branch" command.)

[stefanhaller]

Although, wait a moment: we could totally do the check on our side (in what we think is a better way than git does it), and always use -D. What do you think? (To be clear, this is unrelated to this PR, I'm talking about a general improvement to the "delete local branch" command.)

After a little mailing list discussion, I'm now convinced that this is what we should do: check if the local branch is contained either in current HEAD, or in its own upstream, or in any of our main branches, and if it isn't, prompt the user for confirmation. Then (either way), use -D to delete the branch.

I'll work on a PR soon. We can then rebuild this one on top of that.

[stefanhaller]

The PR I was talking about above is here: #3915.

I then went ahead and also added the "delete both" menu entry on top of it, since it was only a very small step from there: #3916.

[stefanhaller]

Superseded by #3916.