On Tue, Aug 11, 2015 at 7:28 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>> Remove unnecessary variables from ref_list and ref_item which were
>> used for width computation. This is to make ref_item similar to
>> ref-filter's ref_array_item. This will ensure a smooth port of
>> branch.c to use ref-filter APIs in further patches.
>>
>> Previously the maxwidth was computed when inserting the refs into the
>> ref_list. Now, we obtain the entire ref_list and then compute
>> maxwidth.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
>> ---
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 4fc8beb..b058b74 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>> -static int calc_maxwidth(struct ref_list *refs)
>> +static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
>> {
>> - int i, w = 0;
>> + int i, max = 0;
>> for (i = 0; i < refs->index; i++) {
>> + struct ref_item *it = &refs->list[i];
>> + int w = utf8_strwidth(it->name);
>> +
>> if (refs->list[i].ignore)
>> continue;
>
> Nit: Unnecessary work. You compute the width and then immediately
> throw it away when 'ignore' is true.
>
> Also, you use 'it' elsewhere rather than 'refs->list[i]' but not this
> line, which makes it seem out of place. I would have expected to see:
>
> if (it->ignore)
> continue;
>
> (Despite the noisier diff, the end result will be more consistent.)
>
Ah right, will put that after the check :D
Yea, that seems out of place. Thanks
--
Regards,
Karthik Nayak
--
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