[PATCH] index-pack: show chain length histogram as graph for better visual

[PATCH] index-pack: show chain length histogram as graph for better visual

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 definitely better than raw numbers but not really important

 builtin/index-pack.c | 11 ++---------
 diff.c               | 35 +++++++++++++++++++++++++++++++++--
 diff.h               |  2 ++
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2f37a38..a875894 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -11,6 +11,7 @@
 #include "exec_cmd.h"
 #include "streaming.h"
 #include "thread-utils.h"
+#include "diff.h"
 
 static const char index_pack_usage[] =
 "git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
@@ -1476,15 +1477,7 @@ static void show_pack_info(int stat_only)
 			     "non delta: %d objects",
 			     baseobjects),
 			  baseobjects);
-	for (i = 0; i < deepest_delta; i++) {
-		if (!chain_histogram[i])
-			continue;
-		printf_ln(Q_("chain length = %d: %lu object",
-			     "chain length = %d: %lu objects",
-			     chain_histogram[i]),
-			  i + 1,
-			  chain_histogram[i]);
-	}
+	show_histogram_graph("chain length = ", chain_histogram, deepest_delta);
 }
 
 int cmd_index_pack(int argc, const char **argv, const char *prefix)
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;
 	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));
+	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);
+	for (i = 0; i < nr; i++)
+		free(files[i].name);
+	free(files);
+	free(diffstat.files);
+}
+
 static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
 {
 	int i, adds = 0, dels = 0, total_files = data->nr;
@@ -4548,7 +4579,7 @@ void diff_flush(struct diff_options *options)
 		if (output_format & DIFF_FORMAT_NUMSTAT)
 			show_numstat(&diffstat, options);
 		if (output_format & DIFF_FORMAT_DIFFSTAT)
-			show_stats(&diffstat, options);
+			show_stats(&diffstat, options, 1);
 		if (output_format & DIFF_FORMAT_SHORTSTAT)
 			show_shortstats(&diffstat, options);
 		if (output_format & DIFF_FORMAT_DIRSTAT)
diff --git a/diff.h b/diff.h
index ce123fa..d09b937 100644
--- a/diff.h
+++ b/diff.h
@@ -352,4 +352,6 @@ extern int print_stat_summary(FILE *fp, int files,
 			      int insertions, int deletions);
 extern void setup_diff_pager(struct diff_options *);
 
+extern void show_histogram_graph(const char *label, unsigned long *data, int nr);
+
 #endif /* DIFF_H */
-- 
1.9.0.40.gaa8c3ea

Re: [PATCH] index-pack: show chain length histogram as graph for better visual

[Junio C Hamano]

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);