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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2012, 19:41 UTC
Message-ID: <7vr4xmac6f.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4F44D084.7030308@in.waw.pl>

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

> by "scales monotonically with the change count" I meant with two
> different commits.

Because of the scaling, the same 300-line change to the same file will be shown with different number of +- depending on the amount of change to other files in the same commit. It is not something we should be worried about to begin with, unless you are going to read all commits in advance and find necessary length of the name part, and also maxchange across these commits to align things across commits, which of course would not mesh well with our desire to stream output whenever possible.

>  [7.3/8] limit graph part to 40 columns

Read the problem you are trying to solve again (from your 3/8):

>> If commits changing a lot of lines are displayed in a wide terminal
>> window (200 or more columns), and the +- graph would use the full
>> width, the output would look bad. Messages wrapped to about 80 columns
>> would be interspersed with very long +- lines.

If your name part needs only 10-20 columns and you are used to seeing graph bars that are 60-70 columns long on your standard 80-column terminal, you may see it ugly if the same short names are followed by graph bars that are 180-190 columns (i.e. "very long +-"), when the messages still fit 80 columns on the standard terminal, which is a width that can comfortably be scanned with horizontal movement of human eyes. I would understand that concern, although I do not personally think it is a big deal.

Imagine if your name part were 125 columns wide (200 * 5/8), and also imagine you drew the same graph bars as before, i.e. using 60-70 columns. That line will fill almost the full terminal width, and will extend beyond the right edge of the message block, but you are not showing "very long +- lines" anymore. The length of the line comes mostly from overly long names, carrying more information than before (because your patch lets us show larger parts of such a long name on wider terminal), and without losing much information from the graph part.

Now imagine if your patch only needed 10-20 columns for the names, and you are showing the same change on an 80-column terminal with your 40-column cap. You will introduce a new problem with "very short +-" lines instead, as the "diff --stat" block will be much shorter than the surrounding text.

You fixed a bug in the original partitioning with 1/8, which was caused by a code that capped the length of the graph part too early before taking the width necessary to show the name into account; the bug resulted in a graph that did not fill the allotted space fully to the right, even though the user wanted to give full width to the graph.

The ugliness avoidance against "very long +-" becomes an issue only on wide terminals; doing the same 40-column cap on non-wide terminals is to re-introduce the same "cap the graph width too early, ending up with a graph that is narrower than desired" bug you fixed. Why do you keep insisting on that broken approach?

It might be just the matter of raising the artificial cap to much higher than 40-column (say, 80-column). A possible alternative may be to declare that the perceived ugliness is the user's problem of having overly wide terminal in the first place and do without any such cap.

Either is fine, but regressing output on 80-column terminal when showing a patch with short filenames and large changes is unacceptable, not because I personally use 80-col terminal myself (I don't---mine is a bit wider but not 200), but because it changes behaviour from the old code without any good justification to do so.