Re: [PATCH v9 2/4] checkout: improve message for ambiguous remote branch name

[Junio C Hamano <gitster@pobox.com>]

"Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/checkout.c b/checkout.c
> index 1588b116ee..2806b783ec 100644
> --- a/checkout.c
> +++ b/checkout.c
> @@ -8,6 +8,7 @@
>  #include "checkout.h"
>  #include "config.h"
>  #include "strbuf.h"
> +#include "string-list.h"
>  
>  struct tracking_name_data {
>  	/* const */ char *src_ref;
> @@ -17,6 +18,7 @@ struct tracking_name_data {
>  	const char *default_remote;
>  	char *default_dst_ref;
>  	struct object_id *default_dst_oid;
> +	struct string_list **remote_names;
>  };

Do we really need double indirection?  The unique_tracking_name()
function that uses this struct for callback receives a pointer to
the string list the caller has, and the job of the callback is to
append to the supplied string list.  

It is not like it wants to swap the given pointer to a string list
to the pointer to another string list, so I do not see a reason for
anybody involved in the callchain to want this as a pointer to a
pointer.

>  #define TRACKING_NAME_DATA_INIT { 0 }
> @@ -39,6 +41,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
>  		oidcpy(dst, cb->dst_oid);
>  		cb->default_dst_oid = dst;
>  	}
> +	if (cb->remote_names)
> +		string_list_append(*cb->remote_names, remote->name);

If we lose the double indirection, this can become

		string_list_append(cb->remote_names, remote->name);

>  char *unique_tracking_name(const char *name, struct object_id *oid,
> -			   int *dwim_remotes_matched)
> +			   int *dwim_remotes_matched,
> +			   struct string_list *dwim_remote_names)
>  {
>  	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
>  	const char *default_remote = NULL;
> -	if (!repo_config_get_string_tmp(the_repository, "checkout.defaultremote", &default_remote))
> +
> +	if (!repo_config_get_string_tmp(the_repository,
> +					"checkout.defaultremote",
> +					&default_remote))
>  		cb_data.default_remote = default_remote;
>  	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
>  	cb_data.dst_oid = oid;
> +	if (dwim_remote_names)
> +		cb_data.remote_names = &dwim_remote_names;

And this can become

		cb_data.remote_names = dwim_remote_names;