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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/10/2012 07:15 AM, Nguyen Thai Ngoc Duy wrote:
2012/2/10 Zbigniew Jędrzejewski-Szmek<zbyszek@xxxxxxxxx>:
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]