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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> 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.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]