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

Re: [PATCH v2 2/3] Adjust stat width calculations to take --graph output into account

From
Lucian Poston <lucian.poston@gmail.com>
Date
Apr 16, 2012, 11:04 UTC
Message-ID
<CACz_eyfEpE8nZua3JkYtSV42aR_CKJRwvz=4TbOw2zCqSJuDOw@mail.gmail.com>
In-Reply-To
<4F86ABA7.8080703@in.waw.pl>

On Thu, Apr 12, 2012 at 03:17, Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> wrote:

Show 52 quoted lines
>>>> +
>>>> +             /*
>>>> +              * If the remaining unreserved space will not accomodate
>>>> the
>>>> +              * filenames, adjust name_width to use all available
>>>> remaining space.
>>>> +              * Otherwise, assign any extra space to graph_width.
>>>> +              */
>>>> +             if (name_width>    width - reserved_character_count -
>>>> graph_width) {
>>>> +                     name_width = width - reserved_character_count -
>>>> graph_width;
>>>> +             } else {
>>>> +                     graph_width = width - reserved_character_count -
>>>> name_width;
>>>> +             }
>>>> +
>>>> +             /*
>>>> +              * If stat-graph-width was specified, limit graph_width to
>>>> its value.
>>>> +              */
>>>>               if (options->stat_graph_width&&
>>>> -                 graph_width>    options->stat_graph_width)
>>>> +                             graph_width>    options->stat_graph_width)
>>>> {
>>>>                       graph_width = options->stat_graph_width;
>>>> -             if (name_width>    width - number_width - 6 - graph_width)
>>>> -                     name_width = width - number_width - 6 -
>>>> graph_width;
>>>> -             else
>>>> -                     graph_width = width - number_width - 6 -
>>>> name_width;
>>>> +             }
>>>
>>> Here, the order of the two tests
>>> (1) if (options->stat_graph_width&&  graph_width>
>>>  options->stat_graph_width)
>>>
>>> (2) if (name_width>  width - number_width - 6 - graph_width)
>>> is reversed. This is not OK, because this means that
>>> options->stat_graph_width will be used unconditionally, while
>>> before it was subject to limiting by total width.
>>
>>
>> If options->stat_graph_width is specified, it should always limit the
>> value of graph_width, correct? Since (1) is the last test, it can only
>> decrease the value of graph_width, which would already be limited by
>> the total width.
>
> Right, but the way the tests are ordered now, we could end up decreasing
> name_width first (after (2)) and then graph_width (after (1)), actually
> using less than full width.
Ahh, I didn't think about that.
I just reverted the order back to the original.
>> I just noticed that name_width isn't being limited to stat_name_width,
>> if it is specified. I'll add a check for that.
>
> Sounds good.

FYI, in patch v3, I reverted the check for stat_graph_width as previously mentioned, and I ended up not adding a check for stat_name_width. It remains the case that in certain scenarios, name_width & graph_width could be set to values greater than stat_name_width & stat_graph_width.

Previous: Zbigniew Jędrzejewski-SzmekNext: Junio C Hamano
Message 11 of 15 in “Add output_prefix_length to diff_options”
  1. 1/3 Add output_prefix_length to diff_optionsLucian Poston, Mar 22, 2012
  2. 2/3 Adjust stat width calculations to take --graph output into accountLucian Poston, Mar 22, 2012
  3. Zbigniew Jędrzejewski-SzmekMar 22, 2012
  4. Lucian PostonMar 23, 2012
  5. Junio C HamanoMar 22, 2012
  6. Lucian PostonMar 23, 2012
  7. Lucian PostonMar 23, 2012
  8. Zbigniew Jędrzejewski-SzmekMar 23, 2012
  9. Lucian PostonApr 12, 2012
  10. Zbigniew Jędrzejewski-SzmekApr 12, 2012
  11. Lucian PostonApr 16, 2012
  12. Junio C HamanoMar 23, 2012
  13. Lucian PostonApr 12, 2012
  14. 3/3 t4052: Test that stat width is adjusted for prefixesLucian Poston, Mar 22, 2012
  15. Lucian PostonMar 23, 2012

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.