{"thread":{"id":"35931","subject":"[PATCH] index-pack: show chain length histogram as graph for better visual","startedAt":"2014-02-22T01:28:24Z","lastAt":"2014-02-24T17:27:42Z","messageCount":2,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235161","messageId":"1393032504-11854-1-git-send-email-pclouds@gmail.com","threadId":"35931","inReplyTo":null,"subject":"[PATCH] index-pack: show chain length histogram as graph for better visual","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-22T01:28:24Z","receivedAt":"2014-02-22T01:28:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n definitely better than raw numbers but not really important\n\n builtin/index-pack.c | 11 ++---------\n diff.c               | 35 +++++++++++++++++++++++++++++++++--\n diff.h               |  2 ++\n 3 files changed, 37 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2f37a38..a875894 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -11,6 +11,7 @@\n #include \"exec_cmd.h\"\n #include \"streaming.h\"\n #include \"thread-utils.h\"\n+#include \"diff.h\"\n \n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n@@ -1476,15 +1477,7 @@ static void show_pack_info(int stat_only)\n \t\t\t     \"non delta: %d objects\",\n \t\t\t     baseobjects),\n \t\t\t  baseobjects);\n-\tfor (i = 0; i < deepest_delta; i++) {\n-\t\tif (!chain_histogram[i])\n-\t\t\tcontinue;\n-\t\tprintf_ln(Q_(\"chain length = %d: %lu object\",\n-\t\t\t     \"chain length = %d: %lu objects\",\n-\t\t\t     chain_histogram[i]),\n-\t\t\t  i + 1,\n-\t\t\t  chain_histogram[i]);\n-\t}\n+\tshow_histogram_graph(\"chain length = \", chain_histogram, deepest_delta);\n }\n \n int cmd_index_pack(int argc, const char **argv, const char *prefix)\ndiff --git a/diff.c b/diff.c\nindex 8e4a6a9..ca2b92a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1489,7 +1489,8 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)\n \treturn ret;\n }\n \n-static void show_stats(struct diffstat_t *data, struct diff_options *options)\n+static void show_stats(struct diffstat_t *data, struct diff_options *options,\n+\t\t       int summary)\n {\n \tint i, len, add, del, adds = 0, dels = 0;\n \tuintmax_t max_change = 0, max_len = 0;\n@@ -1729,10 +1730,40 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n \t\textra_shown = 1;\n \t}\n+\tif (!summary)\n+\t\treturn;\n \tfprintf(options->file, \"%s\", line_prefix);\n \tprint_stat_summary(options->file, total_files, adds, dels);\n }\n \n+void show_histogram_graph(const char *label, unsigned long *data, int nr)\n+{\n+\tstruct diffstat_t diffstat;\n+\tstruct diff_options options;\n+\tstruct diffstat_file *files;\n+\tchar buf[64];\n+\tint i;\n+\n+\tmemset(&options, 0, sizeof(options));\n+\toptions.file = stdout;\n+\toptions.use_color = diff_use_color_default;\n+\toptions.stat_width = -1;\n+\tdiffstat.nr = nr;\n+\tdiffstat.files = xmalloc(nr * sizeof(*diffstat.files));\n+\tfiles = xcalloc(nr, sizeof(*files));\n+\tfor (i = 0; i < nr; i++) {\n+\t\tdiffstat.files[i] = files + i;\n+\t\tsprintf(buf, \"%s %d\", label, i);\n+\t\tfiles[i].name = xstrdup(buf);\n+\t\tfiles[i].added = data[i];\n+\t}\n+\tshow_stats(&diffstat, &options, 0);\n+\tfor (i = 0; i < nr; i++)\n+\t\tfree(files[i].name);\n+\tfree(files);\n+\tfree(diffstat.files);\n+}\n+\n static void show_shortstats(struct diffstat_t *data, struct diff_options *options)\n {\n \tint i, adds = 0, dels = 0, total_files = data->nr;\n@@ -4548,7 +4579,7 @@ void diff_flush(struct diff_options *options)\n \t\tif (output_format & DIFF_FORMAT_NUMSTAT)\n \t\t\tshow_numstat(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_DIFFSTAT)\n-\t\t\tshow_stats(&diffstat, options);\n+\t\t\tshow_stats(&diffstat, options, 1);\n \t\tif (output_format & DIFF_FORMAT_SHORTSTAT)\n \t\t\tshow_shortstats(&diffstat, options);\n \t\tif (output_format & DIFF_FORMAT_DIRSTAT)\ndiff --git a/diff.h b/diff.h\nindex ce123fa..d09b937 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -352,4 +352,6 @@ extern int print_stat_summary(FILE *fp, int files,\n \t\t\t      int insertions, int deletions);\n extern void setup_diff_pager(struct diff_options *);\n \n+extern void show_histogram_graph(const char *label, unsigned long *data, int nr);\n+\n #endif /* DIFF_H */\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"235269","messageId":"xmqq8ut0beup.fsf@gitster.dls.corp.google.com","threadId":"35931","inReplyTo":"1393032504-11854-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] index-pack: show chain length histogram as graph for better visual","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-24T17:27:42Z","receivedAt":"2014-02-24T17:27:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  definitely better than raw numbers but not really important\n\nI would appreciate if it were an optional feature.  \n\n> diff --git a/diff.c b/diff.c\n> index 8e4a6a9..ca2b92a 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1489,7 +1489,8 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)\n>  \treturn ret;\n>  }\n>  \n> -static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> +static void show_stats(struct diffstat_t *data, struct diff_options *options,\n> +\t\t       int summary)\n>  {\n>  \tint i, len, add, del, adds = 0, dels = 0;\n>  \tuintmax_t max_change = 0, max_len = 0;\n> @@ -1729,10 +1730,40 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>  \t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n>  \t\textra_shown = 1;\n>  \t}\n> +\tif (!summary)\n> +\t\treturn;\n\nYuck.\n\n>  \tfprintf(options->file, \"%s\", line_prefix);\n>  \tprint_stat_summary(options->file, total_files, adds, dels);\n>  }\n>  \n> +void show_histogram_graph(const char *label, unsigned long *data, int nr)\n> +{\n> +\tstruct diffstat_t diffstat;\n> +\tstruct diff_options options;\n> +\tstruct diffstat_file *files;\n> +\tchar buf[64];\n> +\tint i;\n> +\n> +\tmemset(&options, 0, sizeof(options));\n> +\toptions.file = stdout;\n> +\toptions.use_color = diff_use_color_default;\n> +\toptions.stat_width = -1;\n> +\tdiffstat.nr = nr;\n> +\tdiffstat.files = xmalloc(nr * sizeof(*diffstat.files));\n\nDouble yuck for an incomplete refactoring.  Why should a generic\nhistogram-drawer know about *diff*-anything (and if this is still\na diff-specific histogram-drawer, verify-pack has no business\ncalling it)?\n\nI like the idea very much, but not this particular execution.\n\n> +\tfiles = xcalloc(nr, sizeof(*files));\n> +\tfor (i = 0; i < nr; i++) {\n> +\t\tdiffstat.files[i] = files + i;\n> +\t\tsprintf(buf, \"%s %d\", label, i);\n> +\t\tfiles[i].name = xstrdup(buf);\n> +\t\tfiles[i].added = data[i];\n> +\t}\n> +\tshow_stats(&diffstat, &options, 0);\n"}]}