Johannes Schindelin <johannes.schinde...@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.

Reply via email to