Better branch delete confirmation

[stefanhaller]

• PR Description

When deleting a local branch, put up the "This branch is not fully merged, do you want to force delete it" confirmation only when the branch is not merged into any of the main branches.

• 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)
• If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
• Docs have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 8a328a51	✅ 93.33%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (8a328a5)	Report Missing	Report Missing	Report Missing
Head commit (c712b1d)	50491	43400	85.96%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3915)	165	154	93.33%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[jesseduffield]

Just to be clear on the change:

• previous behaviour: try to delete the branch with force: false and if git complains, prompt the user to confirm. Git will complain if the commits on the branch can't be found in any other branch, regardless of whether it's a main branch or not.
• new behaviour: check if the branch is merged to any of the main branches, and if not, prompt the user to confirm. Either way, do a force delete.

So we will now be more likely than before to prompt the user for confirmation because we're reducing the number of branches we look for merged commits.

Have I got that right?

Code seems fine but I want to know: what's the motivation behind this change?

[stefanhaller]

No, it's the other way round. Previously, we would be prompted more often. I wish git would complain only if the commits are not contained in any other branch, and I proposed this on the mailing list, but the maintainers had reasons why they don't want that. I explained this a bit more in the commit message of the last commit.

The main scenario that this improves is that you look at a local branch that was just merged and deleted, so you see "upstream gone". With git's logic, when you try to delete the branch while another, unrelated branch is checked out (or if main is checked out but you didn't pull it yet), you are prompted that the branch wasn't merged yet. Which doesn't make sense, it was just merged, just not to the currently checked out head.

With all that said, unfortunately this fix still doesn't solve our workflow at work. We have a merge bot that rebases the branch onto main before merging, and traffic in our repo is so high that this happens pretty much every time; so I still get the warning when I delete my local branch. So far I couldn't think of anything that we could do to improve that.

[jesseduffield]

Ah, that makes sense. In that case LGTM