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

From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Aug 10, 2026, 13:07 UTC
Message-ID: <CALnO6CAdr0ft8KFgGCFX9ueKUdX9-2DwB+SNs3Q8ykw4ne=54Q@mail.gmail.com>
In-Reply-To: <pull.2197.git.1786177301832.gitgitgadget@gmail.com>

Hi Yoichi,

On Sat, Aug 8, 2026 at 4:21 AM Yoichi NAKAYAMA via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 10 quoted lines

>
> From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
>
> Display a descriptive message when DWIM fails.
>
> Add advice on how to work around this by specifying the fully
> qualified name or by setting checkout.defaultRemote.
>
> Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
> ---

[snip]

Show 21 quoted lines

> -static char *dwim_branch(const char *path, char **new_branch)
> +static char *dwim_branch(const struct add_opts *opts, const char *path, char **new_branch)
>  {
>         int n;
>         int branch_exists;
> @@ -781,8 +791,14 @@ static char *dwim_branch(const char *path, char **new_branch)
>
>         *new_branch = branchname;
>         if (guess_remote) {
> +               int num_matches = 0;
>                 struct object_id oid;
> -               char *remote = unique_tracking_name(*new_branch, &oid, NULL);
> +               char *remote = unique_tracking_name(*new_branch, &oid, &num_matches);
> +               if (!opts->quiet && !remote && num_matches > 1) {
> +                       if (advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME))
> +                               advise(_(message_advice_ambiguous_remote_tracking_branch));
> +                       warning(_("'%s' matched multiple (%d) remote tracking branches\n"), branchname, num_matches);
> +               }
>                 return remote;
>         }
>         return NULL;

I suppose the extra warning won't hurt anyone's workflow :) so that's good.

[snip]

Show 17 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.

Thanks!

-- 
D. Ben Knoble