git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 02/10] branch: refactor width computation

From
Karthik Nayak <karthik.188@gmail.com>
Date
Aug 11, 2015, 13:10 UTC
Message-ID
<CAOLa=ZQSm+YRHVUGxrKDLHaUu4jT82XSLaH-BBf-cL0KNNZ82A@mail.gmail.com>
In-Reply-To
<CAPig+cRHvkAEM1ayVc5nj6RCih52x1+Bn9YTQPV93ej=3arK2Q@mail.gmail.com>
On Tue, Aug 11, 2015 at 7:28 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 40 quoted lines
> On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak <karthik.188@gmail.com> 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@gmail.com>
>> ---
>> 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
Previous: Eric SunshineNext: Karthik Nayak
Message 13 of 27 in “Port branch.c to ref-filter.”
  1. 0/10 Port branch.c to ref-filter.Karthik Nayak, Aug 4, 2015
  2. 01/10 ref-filter: add option to filter only branchesKarthik Nayak, Aug 4, 2015
  3. Junio C HamanoAug 11, 2015
  4. Karthik NayakAug 13, 2015
  5. Karthik NayakAug 13, 2015
  6. Karthik NayakAug 13, 2015
  7. Junio C HamanoAug 14, 2015
  8. Karthik NayakAug 14, 2015
  9. Matthieu MoyAug 13, 2015
  10. Karthik NayakAug 13, 2015
  11. 02/10 branch: refactor width computationKarthik Nayak, Aug 4, 2015
  12. Eric SunshineAug 11, 2015
  13. Karthik NayakAug 11, 2015
  14. 03/10 branch: bump get_head_description() to the topKarthik Nayak, Aug 4, 2015
  15. Eric SunshineAug 11, 2015
  16. Karthik NayakAug 11, 2015
  17. 04/10 branch: roll show_detached HEAD into regular ref_listKarthik Nayak, Aug 4, 2015
  18. Eric SunshineAug 11, 2015
  19. Karthik NayakAug 11, 2015
  20. 05/10 branch: move 'current' check down to the presentation layerKarthik Nayak, Aug 4, 2015
  21. 06/10 branch: drop non-commit error reportingKarthik Nayak, Aug 4, 2015
  22. 07/10 branch.c: use 'ref-filter' data structuresKarthik Nayak, Aug 4, 2015
  23. 08/10 branch.c: use 'ref-filter' APIsKarthik Nayak, Aug 4, 2015
  24. 09/10 branch: add '--points-at' optionKarthik Nayak, Aug 4, 2015
  25. Karthik NayakAug 4, 2015
  26. Junio C HamanoAug 5, 2015
  27. Karthik NayakAug 7, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.