Re: [PATCH v4 8/9] checkout: add advice for ambiguous "checkout <branch>"

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2018, 04:32 UTC
Message-ID: <xmqqin7386tv.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180531195252.29173-9-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 15 quoted lines

> @@ -1269,6 +1270,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  	if (opts.patch_mode || opts.pathspec.nr) {
>  		int ret = checkout_paths(&opts, new_branch_info.name,
>  					 &dwim_remotes_matched);
> +		if (ret && dwim_remotes_matched > 1 &&
> +		    advice_checkout_ambiguous_remote_branch_name)
> +			advise(_("The argument '%s' matched more than one remote tracking branch.\n"
> +				 "We found %d remotes with a reference that matched. So we fell back\n"
> +				 "on trying to resolve the argument as a path, but failed there too!\n"
> +				 "\n"
> +				 "Perhaps you meant fully qualify the branch name? E.g. origin/<name>\n"
> +				 "instead of <name>?"),
> +			       argv[0],
> +			       dwim_remotes_matched);
>  		return ret;

Do we give "checkout -p no-such-file" the above wall of text?

Somehow checkout_paths(), which is "we were given a tree-ish and pathspec and told to grab the matching paths out of it and stuff them to the index and the working tree", is a wrong place to be doing the "oh, what the caller thought was pathspec may turn out to be a rev, so check that too for such a confused caller". Shouldn't the caller be doing all that (which would mean we wan't need to pass "remotes-matched" to the function, as the helper has nothing to do with deciding which arg is the tree-ish).