All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	git@vger.kernel.org, gitster@pobox.com
Cc: Michael J Gruber <git@drmicha.warpmail.net>
Subject: Re: [PATCH 3/4] diff --stat: use the real terminal width
Date: Fri, 10 Feb 2012 12:25:14 +0100	[thread overview]
Message-ID: <4F34FE9A.7020600@in.waw.pl> (raw)
In-Reply-To: <CACsJy8APGeTNv_E3qD=xFCiLC25M_nm3aJbq6YU73J=X0Wxh2w@mail.gmail.com>

On 02/10/2012 07:15 AM, Nguyen Thai Ngoc Duy wrote:
> 2012/2/10 Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>:
>> @@ -1341,7 +1342,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>>                 line_prefix = msg->buf;
>>         }
>>
>> -       width = options->stat_width ? options->stat_width : 80;
>> +       width = options->stat_width ? options->stat_width : term_columns();
>>         name_width = options->stat_name_width ? options->stat_name_width : 50;
>>         count = options->stat_count ? options->stat_count : data->nr;
>
> I tried this in the past and "git log -p" looked ugly on git.git
> mainly because commit messages are still ~70 char long lines on my 279
> char wide terminal. If this is project dependent, perhaps a config
> key? Also the "50" below the changed line, maybe you want to change it
> to 0.6 * width.

Thanks for all the comments. I'll post a newer version, but I have two 
questions:

I agree that making the output very wide with lots of +- is not very 
elegant. (E.g. 8f24a6323ece9be1bf1a04b4b5856112438337f2 has
    builtin/grep.c |  142 +++--------------------------------....--
which doesn't look right.). So I think it would make sense to limit
the graph part to something like 50 columns, even if there's more space.
I believe that git.git would look fine with this change. There are some 
fairly long lines 
(t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master 
is 86 chars) but with 50 columns of graph the output would take 140 
columns -- with the graph part slightly sticking out from the 80 column 
descriptions, but still not too ugly.

Should I add a new option --stat-graph-width in analogy to 
--stat-name-width, or should this be hard-coded?

JC:
 > The output from "git format-patch" shouldn't be affected at all by the
 > width of the terminal the patch sender happened to have used when the
 > command was run when the user did not explicitly ask a custom width by
 > giving a --stat-width command line option.
 >
 > How do you prevent regression to the command in this series?
git format-patch is not affected by default. But with --stdout
the width is changed, iff stdout is a tty. When --stdout output
is connected to a pipe, the width is not changed. I think that
this behaviour is OK.

Should a test be added to check that 'git format-patch --stat' output 
doesn't change? Should I test for something else?

--
Zbyszek

  reply	other threads:[~2012-02-10 11:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-09 23:58 (unknown), Zbigniew Jędrzejewski-Szmek
2012-02-09 23:58 ` [PATCH 1/4] Move git_version_string to help.c in preparation for diff changes Zbigniew Jędrzejewski-Szmek
2012-02-10  0:46   ` Junio C Hamano
2012-02-09 23:58 ` [PATCH 2/4] help.c: make term_columns() cached and export it Zbigniew Jędrzejewski-Szmek
2012-02-10  0:50   ` Junio C Hamano
2012-02-09 23:58 ` [PATCH 3/4] diff --stat: use the real terminal width Zbigniew Jędrzejewski-Szmek
2012-02-10  0:54   ` Junio C Hamano
2012-02-10  6:15   ` Nguyen Thai Ngoc Duy
2012-02-10 11:25     ` Zbigniew Jędrzejewski-Szmek [this message]
2012-02-10 13:00       ` Nguyen Thai Ngoc Duy
2012-02-10 16:39         ` [PATCH 0/3 v2] " Zbigniew Jędrzejewski-Szmek
2012-02-10 16:39           ` [PATCH 1/3] Move git_version_string to help.c before diff changes Zbigniew Jędrzejewski-Szmek
2012-02-10 17:58             ` Junio C Hamano
2012-02-10 16:39           ` [PATCH 2/3] help.c: make term_columns() cached and export it Zbigniew Jędrzejewski-Szmek
2012-02-11  4:36             ` Nguyen Thai Ngoc Duy
2012-02-11 10:49               ` Zbigniew Jędrzejewski-Szmek
2012-02-12  9:40                 ` Junio C Hamano
2012-02-12 14:12                   ` [PATCH 1/2] Save terminal width before setting up pager and export term_columns() Zbigniew Jędrzejewski-Szmek
2012-02-13 23:00                     ` Junio C Hamano
2012-02-14 11:44                     ` Nguyen Thai Ngoc Duy
2012-02-14 11:53                       ` Zbigniew Jędrzejewski-Szmek
2012-02-12 14:16                   ` [PATCH 2/2] Rename lineno_width to decimal_width and export it Zbigniew Jędrzejewski-Szmek
2012-02-13 23:29                     ` Junio C Hamano
2012-02-14 12:24                       ` [PATCH v2] make lineno_width() from blame reusable for others Zbigniew Jędrzejewski-Szmek
2012-02-10 16:39           ` [PATCH 3/3] diff --stat: use the real terminal width Zbigniew Jędrzejewski-Szmek
2012-02-10 18:24           ` [PATCH 0/3 v2] " Junio C Hamano
2012-02-12 14:30             ` [PATCH v3] diff --stat: use the full " Zbigniew Jędrzejewski-Szmek
2012-02-14  1:08               ` Junio C Hamano
2012-02-14 23:45                 ` [PATCH 1/3 v4] " Zbigniew Jędrzejewski-Szmek
2012-02-14 23:45                   ` [PATCH 2/3 v4] diff --stat: better alignment for binary files Zbigniew Jędrzejewski-Szmek
2012-02-14 23:45                   ` [PATCH 3/3 v4] Update diff --stat output in tests and tutorial Zbigniew Jędrzejewski-Szmek
2012-02-15  1:21                     ` Junio C Hamano
2012-02-15 11:03                       ` [PATCH 1/3 v5] diff --stat: tests for long filenames and big change counts Zbigniew Jędrzejewski-Szmek
2012-02-15 11:03                         ` [PATCH 2/3 v5] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-02-15 18:07                           ` Junio C Hamano
2012-02-15 11:03                         ` [PATCH 3/3 v5] diff --stat: use less columns for change counts Zbigniew Jędrzejewski-Szmek
2012-02-15 12:12                           ` Nguyen Thai Ngoc Duy
2012-02-15 17:12                         ` [PATCH 1/3 v5] diff --stat: tests for long filenames and big " Junio C Hamano
2012-02-15 17:33                           ` Junio C Hamano
2012-02-16  9:57                           ` Zbigniew Jędrzejewski-Szmek
2012-02-16 20:01                             ` Junio C Hamano
2012-02-15  0:07                   ` [PATCH 1/3 v4] diff --stat: use the full terminal width Junio C Hamano
2012-02-15  1:18                   ` Junio C Hamano
2012-02-15  7:39                   ` Johannes Sixt
2012-02-09 23:58 ` [PATCH 4/4] diff --stat: use most of the space for file names Zbigniew Jędrzejewski-Szmek
2012-02-10  0:55   ` 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=4F34FE9A.7020600@in.waw.pl \
    --to=zbyszek@in.waw.pl \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.