On Thu, Jan 18, 2018 at 5:05 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
> [...]
> The new option --compact-summary implements this with a tweak to support
> mode change, which is shown in --summary too.
>
> For mode changes, executable bit is denoted as "M+x" or "M-x" when it's
> added or removed respectively. The same for when a regular file is
> replaced with a symlink "M+l" or the other way "M-l". This also applies
> to new files. New regulare files are "A", while new executable files or
s/regulare/regular/
> symlinks are "A+x" or "A+l".
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> @@ -188,6 +188,17 @@ and accumulating child directory counts in the parent directories:
> +--compact-summary::
> + Output a condensed summary of extended header information in
> + front of the file name part of diffstat. This option is
> + ignored if --stat is not specified.
Rather than ignoring this option if --stat is not specified, a
different approach would be for --compact-summary to imply --stat.
Also, per documentation:
--stat[=<width>[,<name-width>[,<count>]]]::
These parameters can also be set individually with `--stat-width=<width>`,
`--stat-name-width=<name-width>` and `--stat-count=<count>`.
One wonders if "compact" could be another modifier recognized by --stat.
(Genuine questions/observations; I haven't formed strong opinions either way.)
> diff --git a/diff.c b/diff.c
> @@ -2287,6 +2289,18 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
> + for (i = 0; (i < count) && (i < data->nr); i++) {
Noisy extra parentheses...
for (i = 0; i < count && i < data->nr; i++) {
perhaps? (Not at all worth a re-roll.)
> + const struct diffstat_file *file = data->files[i];
> + int len;
> +
> + if (!file->status_code)
> + continue;
> + len = strlen(file->status_code) + 1;
The +1 is for the space following the status code? (Reading ahead,
that seems to be the case.)
len = strlen(file->status_code) + strlen(" ");
perhaps? (Probably not worth a re-roll.)
> + if (len > max_status_len)
> + max_status_len = len;
> + }
> +
> @@ -2383,6 +2397,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
> options->stat_name_width < max_len) ?
> options->stat_name_width : max_len;
>
> + name_width += max_status_len;
I wonder if it would be clearer to account for the space after the the
status code here rather than above when it was not obvious what +1 was
for.
name_width += max_status_len + strlen(" ");
(and drop the earlier +1)
