From: Zbigniew Jędrzejewski-Szmek Date: Fri, 10 Feb 2012 11:25:14 GMT Subject: Re: [PATCH 3/4] diff --stat: use the real terminal width Message-ID: <4F34FE9A.7020600@in.waw.pl> In-Reply-To: On 02/10/2012 07:15 AM, Nguyen Thai Ngoc Duy wrote: > 2012/2/10 Zbigniew Jędrzejewski-Szmek: >> @@ -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