git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Michael J Gruber <git@drmicha.warpmail.net>,
	pclouds@gmail.com, j.sixt@viscovery.net
Subject: [PATCH v7 0/11] diff --stat: use the full terminal width
Date: Fri, 24 Feb 2012 21:31:16 +0100	[thread overview]
Message-ID: <4F47F394.5070007@in.waw.pl> (raw)
In-Reply-To: <7vr4xmac6f.fsf@alter.siamese.dyndns.org>

Hi,

I accept your reasoning that a graph narrower than 80 columns would not 
look good.

On 02/22/2012 08:41 PM, Junio C Hamano wrote:
> 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.

I've taken both options :) The graph-width-limit is turned into an 
option (commandline --stat-graph-width and config diff.statGraphWidth), 
off by default. Now no tests need to be changed. But if somebody doesn't 
like very wide graph output, they can set the config option to something 
like 80 and forget about this.

This is on top of today's next. I'm sending 1/11 'make lineno_width() 
from blame reusable for others' again, because the version in next is 
the previous version before s/int/uintmax_t/.

Changes since v6:

- 40 column limit for the graph part is changed in an optional, 
configurable limit, off by default
- parts 7.1/8, 7.2/8, 7.3/8 get their own numbers, and 7.3 is split into 
two parts (9/10 is --stat-graph-width and 10/10 is diff.statGraphWidth).

v6:
[1/8]  make lineno_width() from blame reusable for others
   This is very close to what was in pu, but I'm sending a new version:
   - the function argument is changed from int to uintmax_t
     (max_change is uintmax_t and 9/9 does decimal_width(max_change).)
[2/8] diff --stat: tests for long filenames and big change counts
   - Tests are run for format-patch, diff, log, show, and merge.
   - Since tests are not only for format-patch, they are added in a new
     file t/t4052-stat-output.sh.
[3/8] diff --stat: use the full terminal width
   Add logic to use term_columns() when diffopts.stat_width==-1 and
   turn it on in git-diff --stat.
   - show_stats() output is adapted to full terminal width only when
     diffopts.stat_width==-1.
[4/8] show --stat: use the full terminal width
   Enable for git-show.
[5/8] log --stat: use the full terminal width
   Enable for log-show.
[6/8] merge --stat: use the full terminal width
   Enable for git-merge.
[7/8] diff --stat: limit graph part to 40 columns
   Change the logic to divide columns. This part is the unchanged from
   v5, just separated from 3/9.
[8/8] diff --stat: use less columns for change counts
   This one is optional, to be applied or not, "when the dust settles".

v5:
- tests are moved to an earlier patch
   - seq is replaced with a while loop for windows compatibility
   - grep -m 1 is replaced with grep " | "
   - redirects are made portable
   - piped output is split into two commands to verify that the first 
command
     sucessfully runs to completion
- using decimal_width(change count) is moved to a later patch
- "histogram" is really not used


v4:
- comments are updated and the word "histogram" is banished
- "mopping up" is removed (but the minimum width are guaranteed)

v3:
- use decimal_width(max_change) to calculate number of columns
   required for change counts
- rework the logic to divide columns
- document the logic in comments, update docs
- add more tests

v2:
- style fixes
- some tests for git-format-patch added
- patches 3 and 4 squashed together, since they touch the same lines
- graph width is limited to 40 columns, even if there's more space
- patch descriptions extended and cleared up

  reply	other threads:[~2012-02-24 20:31 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-20 21:57 [PATCH 0/8 v6] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 1/8 v6] make lineno_width() from blame reusable for others Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 2/8 v6] diff --stat: tests for long filenames and big change counts Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 3/8 v6] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 4/8 v6] show " Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 5/8 v6] log " Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 6/8 v6] merge " Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 7/8 v6] diff --stat: limit graph part to 40 columns Zbigniew Jędrzejewski-Szmek
2012-02-20 21:57 ` [PATCH 8/8 v6] diff --stat: use less columns for change counts Zbigniew Jędrzejewski-Szmek
2012-02-20 23:41 ` [PATCH 0/8 v6] diff --stat: use the full terminal width Junio C Hamano
2012-02-21  0:26   ` Zbigniew Jędrzejewski-Szmek
2012-02-21  7:05     ` Junio C Hamano
2012-02-21 10:05       ` Zbigniew Jędrzejewski-Szmek
2012-02-21 20:10         ` Junio C Hamano
2012-02-22 11:24           ` Zbigniew Jędrzejewski-Szmek
2012-02-22 11:51             ` [PATCH 7.1/8] diff --stat: use a maximum of 5/8 for the filename part Zbigniew Jędrzejewski-Szmek
2012-02-22 11:51               ` [PATCH 7.2/8] diff --stat: add a test for output with COLUMNS=40 Zbigniew Jędrzejewski-Szmek
2012-02-22 11:51               ` [PATCH 7.3/8] diff --stat: limit graph part to 40 columns Zbigniew Jędrzejewski-Szmek
2012-02-22 19:41             ` [PATCH 0/8 v6] diff --stat: use the full terminal width Junio C Hamano
2012-02-24 20:31               ` Zbigniew Jędrzejewski-Szmek [this message]
2012-02-24 20:32                 ` [PATCH v7 01/11] make lineno_width() from blame reusable for others Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 02/11] diff --stat: tests for long filenames and big change counts Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 03/11] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 04/11] show " Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 05/11] log " Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 06/11] merge " Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 07/11] diff --stat: use a maximum of 5/8 for the filename part Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 08/11] diff --stat: add a test for output with COLUMNS=40 Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 09/11] diff --stat: enable limiting of the graph part Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 10/11] diff --stat: add config option to limit graph width Zbigniew Jędrzejewski-Szmek
2012-02-24 20:32                   ` [PATCH v7 11/11] diff --stat: use less columns for change counts Zbigniew Jędrzejewski-Szmek
2012-02-21 15:16 ` [PATCH 0/8 v6] diff --stat: use the full terminal width Nguyen Thai Ngoc Duy
2012-02-21 16:11   ` Zbigniew Jędrzejewski-Szmek
2012-02-23  7:29     ` Miles Bader
2012-02-23  5:08 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F47F394.5070007@in.waw.pl \
    --to=zbyszek@in.waw.pl \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).