From: Eric Sunshine <sunshine@sunshineco.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH v2] diff: add --compact-summary option to complement --stat
Date: Thu, 18 Jan 2018 13:57:16 -0500 [thread overview]
Message-ID: <CAPig+cQLgs59JYxcmK30qY307ArwqJx6pNOo95Z39_jJ9+D6+g@mail.gmail.com> (raw)
In-Reply-To: <20180118100546.32251-1-pclouds@gmail.com>
On Thu, Jan 18, 2018 at 5:05 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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@gmail.com>
> ---
> 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)
next prev parent reply other threads:[~2018-01-18 18:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-13 13:22 [PATCH/RFC] diff: add --compact-summary option to complement --stat Nguyễn Thái Ngọc Duy
2018-01-13 18:37 ` Philip Oakley
2018-01-14 9:37 ` Simon Ruderich
2018-01-14 10:24 ` Duy Nguyen
2018-01-18 10:05 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2018-01-18 18:57 ` Eric Sunshine [this message]
2018-01-19 0:01 ` Duy Nguyen
2018-01-18 21:23 ` Ævar Arnfjörð Bjarmason
2018-01-19 0:06 ` Duy Nguyen
2018-01-18 22:48 ` Jeff King
2018-01-19 0:26 ` Duy Nguyen
2018-01-19 21:52 ` Jeff King
2018-01-19 21:20 ` Junio C Hamano
2018-01-19 21:53 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPig+cQLgs59JYxcmK30qY307ArwqJx6pNOo95Z39_jJ9+D6+g@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).