From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] index-pack: show chain length histogram as graph for better visual
Date: Mon, 24 Feb 2014 09:27:42 -0800 [thread overview]
Message-ID: <xmqq8ut0beup.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1393032504-11854-1-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Sat, 22 Feb 2014 08:28:24 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail•com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail•com>
> ---
> definitely better than raw numbers but not really important
I would appreciate if it were an optional feature.
> diff --git a/diff.c b/diff.c
> index 8e4a6a9..ca2b92a 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1489,7 +1489,8 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
> return ret;
> }
>
> -static void show_stats(struct diffstat_t *data, struct diff_options *options)
> +static void show_stats(struct diffstat_t *data, struct diff_options *options,
> + int summary)
> {
> int i, len, add, del, adds = 0, dels = 0;
> uintmax_t max_change = 0, max_len = 0;
> @@ -1729,10 +1730,40 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
> fprintf(options->file, "%s ...\n", line_prefix);
> extra_shown = 1;
> }
> + if (!summary)
> + return;
Yuck.
> fprintf(options->file, "%s", line_prefix);
> print_stat_summary(options->file, total_files, adds, dels);
> }
>
> +void show_histogram_graph(const char *label, unsigned long *data, int nr)
> +{
> + struct diffstat_t diffstat;
> + struct diff_options options;
> + struct diffstat_file *files;
> + char buf[64];
> + int i;
> +
> + memset(&options, 0, sizeof(options));
> + options.file = stdout;
> + options.use_color = diff_use_color_default;
> + options.stat_width = -1;
> + diffstat.nr = nr;
> + diffstat.files = xmalloc(nr * sizeof(*diffstat.files));
Double yuck for an incomplete refactoring. Why should a generic
histogram-drawer know about *diff*-anything (and if this is still
a diff-specific histogram-drawer, verify-pack has no business
calling it)?
I like the idea very much, but not this particular execution.
> + files = xcalloc(nr, sizeof(*files));
> + for (i = 0; i < nr; i++) {
> + diffstat.files[i] = files + i;
> + sprintf(buf, "%s %d", label, i);
> + files[i].name = xstrdup(buf);
> + files[i].added = data[i];
> + }
> + show_stats(&diffstat, &options, 0);
prev parent reply other threads:[~2014-02-24 17:28 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-22 1:28 [PATCH] index-pack: show chain length histogram as graph for better visual Nguyễn Thái Ngọc Duy
2014-02-24 17:27 ` Junio C Hamano [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq8ut0beup.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox