Re: [PATCH v23 5/7] branch: add --delete-merged <branch>

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Harald

On 30/07/2026 00:13, Harald Nordgren wrote:
>>> @@ -38,6 +39,7 @@ static const char * const builtin_branch_usage[] = {
>>>        N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
>>>        N_("git branch [<options>] [-r | -a] [--points-at]"),
>>>        N_("git branch [<options>] [-r | -a] [--format]"),
>>> +     N_("git branch [<options>] (--delete-merged <branch>)... [<pattern>...]"),
>>
>> I don't quite follow this - why the "()" and doesn't --delete-merged
>> take a pattern?
> 
> I don't get this one, but would this be better?
> 
> ```
> git branch [--dry-run] --delete-merged <pattern>
>     [--delete-merged <pattern>]... [<branch-pattern>...]

I think I just didn't understand what "(--option)..." means - grepping 
around that does seem to be what we use for "give this option at least 
once" so I think the original is fine.

>>> +static int branch_pushes_to_upstream(struct branch *branch,
>>> +                                  const char *upstream)
>>> +{
>>> +     struct remote *remote = remote_get(remote_for_branch(branch, NULL));
>>> +     char *tracking = NULL;
>>> +     int ret = 0;
>>> +
>>> +     if (remote)
>>> +             tracking = apply_refspecs(&remote->fetch, branch->refname);
>>
>> This tells us which remote tracking ref corresponds to the branch
>>
>>> +     if (tracking && !strcmp(tracking, upstream))
>>> +             ret = 1;
>>
>> Here we check that it does not match the upstream branch. That ignores
>> the push refspect though so does not tell us whether pushing the branch
>> to the upstream remote would update the upstream branch on that remote.
> 
>> We need to apply the push refspec to the local branch, apply the fetch
>> refspec in reverse to the result and then compare that to the upstream
>> branch.

Oops I think that's wrong. We should apply the fetch refspec to the 
result of the push refspec, not apply it in reverse.

If we want to pretend that origin uses "main" rather than "master" we 
could set

	remote.origin.push refs/heads/main:refs/heads/master
	remote.origin.fetch refs/heads/master:refs/remotes/origin/main

so we map refs/heads/main to refs/heads/master and then map that to 
refs/remotes/origin/main. There's nothing (apart from maybe common 
sense) to say the refspecs have to be symmetric though.

> I'll try to do something about this, but I removed a big part of the
> push logic when introducing the stacked branches, it simplified things
> a lot without it. But maybe too simplistic.

Maybe I'm missing something but aren't the two concerns independent?

>>> +             git checkout -b tracks-other other/main --track &&
>>> +             git checkout checked-out &&
>>> +
>>> +             git branch --delete-merged origin/* \
>>> +                     --delete-merged main &&
>>
>> These lines look very short why the wrapping?
> 
> Will unwrap some of these.
> 
>> Why do we keep "lower", rather than clear the upstream config of "mid"?
> 
> We keep lower to preserve the upstream chain of surviving tip.

Yes but why do we want to do that? It's a change in behavior from 
clearing the upstream of mid that wasn't really called out clearly as a 
change in behavior for xxx reason in the cover letter of the iteration 
that introduced it. Why is it desirable to keep the whole chain rather 
than just the upstream of the unmerged branch?

Thanks

Phillip