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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:
>
>> @@ -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).

Well, upon closer inspection adding *dwim_remotes_matched parameter
to checkout_paths() done in an earlier step seems to be totally
bogus and only serves the purpose of confusing reviewers.  The
function does not touch the pointer in any way---it does not use the
pointer to return its findings, and it does not use an earlier
findings to affect its behaviour by dereferencing it.

The dwim_remotes_matched is set by an earlier call to
parse_branchname_arg(), which does gain an int* parameter in this
series.  And that addition _does_ make sense.  That codepath is
where the "do we have many remotes that could match, or none, or
unique?" determination is made.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux