Re: [PATCH] worktree add: improve message for ambiguous remote branch name

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
Date: Aug 10, 2026, 13:35 UTC
Message-ID: <CAF5D8-vkd3GOOpdaC=swLv-gxtCzkdhNNUpbTSwr2PBH_Jk=1w@mail.gmail.com>
In-Reply-To: <CALnO6CAdr0ft8KFgGCFX9ueKUdX9-2DwB+SNs3Q8ykw4ne=54Q@mail.gmail.com>

On Mon, Aug 10, 2026 at 10:08 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:

Show 31 quoted lines

> > @@ -904,10 +920,16 @@ static int add(int ac, const char **av, const char *prefix,
> >
> >                 commit = lookup_commit_reference_by_name(branch);
> >                 if (!commit) {
> > -                       remote = unique_tracking_name(branch, &oid, NULL);
> > +                       int num_matches = 0;
> > +                       remote = unique_tracking_name(branch, &oid, &num_matches);
> >                         if (remote) {
> >                                 new_branch = branch;
> >                                 branch = new_branch_to_free = remote;
> > +                       } else if (num_matches > 1) {
> > +                               if (!opts.quiet && advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME)) {
> > +                                       advise(_(message_advice_ambiguous_remote_tracking_branch));
> > +                               }
> > +                               die(_("'%s' matched multiple (%d) remote tracking branches"), branch, num_matches);
> >                         }
> >                 }
>
> We would now die() here where we didn't before. I'm not suggesting
> that is wrong (I haven't given it much thought), but I was surprised
> to see it in the code without mention in the message, which I've left
> quoted above. In particular, the proposed log message talks about
> giving new advice, so I wasn't expecting us to abort.
>
> Now, it may be that this case already causes an error later on (I
> haven't analyzed that), in which case dying early with a better
> diagnostic is definitely helpful. If that's the case, it would be nice
> to spell that out for the rest of us :)
>
> If not, I would want to know why we can die() here without bothering
> anyone's workflow that is expecting us to carry on.

Before the change, it calles lookup_commit_reference_by_name() again in the if condition and die() at:

    if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
        /* snip */
        die(_("invalid reference: %s"), branch);
    }

The motivation for the fix was that this error message did not accurately reflect the situation.

Thanks,

-- 
Yoichi NAKAYAMA