Re: [PATCH 0/8 v6] diff --stat: use the full terminal width

[Junio C Hamano <gitster@pobox.com>]

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> On 02/21/2012 12:41 AM, Junio C Hamano wrote:
>> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>>
>>> JC:
>>>> Perhaps the maximum for garph_width should be raised to something like
>>>> "min(80, stat_width) - name_width"?
>>> I think that a graph like
>>> a | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> b |    1 -
>>> is not very readable. I like the consistency forced by the 40-column limit.
>>> But I guess that this is very subjective.
>>
>> The above makes it very obvious that there is a huge amount of change made
>> to 'a' and a bit of deletion to 'b', compared to a mini-graph that is
>> truncated to half the screen width.
> Yes. But the same graph with 40 columns tells me exactly the same thing.

That is a bogus argument, isn't it?  You can say the same thing if you
limited the length of the graph bars to 10-columns if you only compare
between 1000 and 1. You can even do with just 5-columns.  For that matter,
without any graph bar at all, it tells us exactly the same thing because
we have numbers.  Does that mean we do not need any bar?  Of course not.
We use bars as visual aid.

Imagine what happens to the graph if you had paths with medium amount of
changes like 980, 800, 40, in addition to 1000 and 1.  By limiting the
length of the bars more than necessary, you are losing the resolution
without a good reason, and that is why I find 40-column limit a poor
design choice.

>> Besides, the above is what you would get without your patch on 80-column
>> terminal, no?
>
> Yes.

I think this "use at most 40-places for the graph bar" was your response
to somebody's observation that "on 200-column terminal, we will still see
the commit log messages (and for many projects, also patch text) that are
designed to be comfortably viewable within the 80-column on the left, and
overlong graph bar stands out like an ugly sore thumb".

While that "ugliness" observation might be a valid one to make, I do not
think limiting the length of the graph bar without taking the length of
the name part into account at all is the right solution to it.

After all, that is exactly the same thinking that led to the bug in the
current code that you fixed with your series, isn't it?  Our safety code
truncated the graph bar width too early without taking the width needed to
show the names into account, and then when the names turn out to be all
short, we ended up wasting space on the right hand side, because we made
the bars too short and the decision was made too early in the code.

If the problem you are addressing is to make sure that the diffstat part
in the series of lines that are structured like this:

   log message part ~80 column
   diff stat part that can extend very very very very very very very long
   patch text part  ~80 column

does not become overly long, wouldn't it be a more natural solution to
make sure that when the total (i.e. name and graph) length can fit to
align with the message and patch (i.e. traditional ~80 col regardless of
the terminal width), not to give it too much width?  If the names are
short, like "a" and "b", that may result in graph bar part to use ~70
columns or so, and if the names are long, like in a Java project, you may
allocate 50 columns to the name and leave only 50 columns or so for the
graph part.

A simple heuristic might be to see if name part (without truncation) and
the graph part (without scaling) fits under 100-columns if the terminal is
wider than that, and if so limit the whole thing to 100-columns before
deciding the allocation of the total into two parts.  If the name part
alone is very wide, showing the name and the graph using the whole
terminal width would give you a better result than using the bars that are
artificially capped to a short limit, I would imagine.