Re: [PATCH v2 1/3] for-each-ref: let upstream/push optionally report the remote name

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

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The implementation chooses *not* to DWIM the push remote if no explicit
> push remote was configured; The reason is that it is possible to DWIM this
> by using
>
> 	%(if)%(push:remotename)%(then)
> 		%(push:remotename)
> 	%(else)
> 		%(upstream:remotename)
> 	%(end)
>
> while it would be impossible to "un-DWIM" the information in case the
> caller is really only interested in explicit push remotes.

Good.

> Note: the dashless, non-CamelCased form `:remotename` follows the
> example of the `:trackshort` example.

Good.  Come to think of it, aside from modifiers, the use of
squashedwords like committerdate and objecttype has long been the
norm in the for-each-ref atom names, so "remotename" is much more in
line with other formats.

> @@ -1354,16 +1371,20 @@ static void populate_value(struct ref_array_item *ref)
>  			if (refname)
>  				fill_remote_ref_details(atom, refname, branch, &v->s);
>  			continue;
> -		} else if (starts_with(name, "push")) {
> +		} else if (atom->u.remote_ref.push) {
>  			const char *branch_name;
>  			if (!skip_prefix(ref->refname, "refs/heads/",
>  					 &branch_name))
>  				continue;
>  			branch = branch_get(branch_name);
>  
> -			refname = branch_get_push(branch, NULL);
> -			if (!refname)
> -				continue;
> +			if (atom->u.remote_ref.push_remote)
> +				refname = NULL;
> +			else {
> +				refname = branch_get_push(branch, NULL);
> +				if (!refname)
> +					continue;
> +			}
>  			fill_remote_ref_details(atom, refname, branch, &v->s);

It still feels a bit strange to pass NULL refname to
fill_remote_ref_details() function, but fixing it would require
overhaul of this if/else if cascade, I think, so let's not worry too
much about it at least for now.

Looks much better than the previous one.  Thanks.  Queued.