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

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

2012/2/10 Zbigniew Jędrzejewski-Szmek[off-list ref]:

quoted

@@ -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