Re: [PATCH 3/4] diff --stat: use the real terminal width

From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Feb 10, 2012, 11:25 UTC
Message-ID: <4F34FE9A.7020600@in.waw.pl>
In-Reply-To: <CACsJy8APGeTNv_E3qD=xFCiLC25M_nm3aJbq6YU73J=X0Wxh2w@mail.gmail.com>

On 02/10/2012 07:15 AM, Nguyen Thai Ngoc Duy wrote:

Show 15 quoted lines

> 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