Phil Hord wrote:
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
>> int eaten, i;
>> unsigned long size;
>> const unsigned char *tagged;
>> + int upstream_present = 0;
>
> This flag is out of place. It should be in the same scope as 'branch'
> since the code which depends on this flag also depends on '!!branch'.
Agreed. Fixed.
> However, I don't think it is even necessary. The only way to reach
> the places where this flag is tested is when (name="upstream") and
> (upstream exists). In all other cases, the parser loops before
> reaching the track/trackshort code or else it doesn't enter it.
Yeah, you're right. I was setting upstream_present in this snippet:
else if (!prefixcmp(name, "upstream")) {
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
If the refname doesn't begin with "refs/heads" in the first place
(which is what I was guarding against), the code will loop and never
reach the track[short] code anyway.
upstream_present factored out now.
>> @@ -698,11 +701,48 @@ static void populate_value(struct refinfo *ref)
>> formatp = strchr(name, ':');
>> /* look for "short" refname format */
>> if (formatp) {
>> + int num_ours, num_theirs;
>> +
>> formatp++;
>> if (!strcmp(formatp, "short"))
>> refname = shorten_unambiguous_ref(refname,
>> warn_ambiguous_refs);
>> - else
>> + else if (!strcmp(formatp, "track") &&
>> + !prefixcmp(name, "upstream")) {
>> + char buf[40];
>> +
>> + if (!upstream_present)
>> + continue;
>> + stat_tracking_info(branch, &num_ours, &num_theirs);
>> + if (!num_ours && !num_theirs)
>> + v->s = "";
>
> Is this the same as 'continue'?
I'll leave this as it is for readability reasons.
Thanks for the review.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html