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

Thanks for all the help to drive this across the finish line!

quoted

quoted

quoted

@@ -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.

Ok, will use the ().

quoted

quoted

quoted

+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

quoted

+     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.

quoted

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'm writing logic for this and an integration test. Thanks!

quoted

quoted

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?

I can do it and get rid of the graph traversal (visited), but I think
we are once again going back to an earlier iteration. One of the other
reviewers wanted to protect transitive branches I think.

Either way is fine, I think it's a pretty small detail in the grand
scheme of things, I just don't want another round of ping-pongs on
this part.


Harald