{"thread":{"id":"27218","subject":"[PATCH 2/2] diff-options.txt: describe --stat-{width,name-width,lines}","startedAt":"2011-04-29T14:57:45Z","lastAt":"2011-05-30T07:36:34Z","messageCount":21,"participants":["Michael J Gruber","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"166737","messageId":"5f16db0f3730be70ff522e63fbd491dc910c34d0.1304089050.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":null,"subject":"[PATCH 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-29T14:57:45Z","receivedAt":"2011-04-29T14:57:45Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Often one is interested in the full --stat output only for commits which\nchange a few files, but not others, because larger restructuring gives a\n--stat which fills a few screens.\n\nIntroduce a new option --stat-lines= which limits the --stat output\nto the first and last lines, separated by a \"...\" line. It can\nalso be given as the third parameter in\n--stat=,,.\n\nAlso, the unstuck form is supported analogous to the other two stat\nparameters.\n\nSigned-off-by: Michael J Gruber \n---\nI would even consider a default of 10 (i.e. show a 20 line stat in full,\nabbreviate larger ones) to be sensible but have refrained from such a\nbehaviour change.\n\nWe have hardcoded defaults for width (80) and name-width (50), so having\none for lines should be okay also. Can I has tis wiz default? ;)\n---\n Documentation/diff-options.txt | 5 ++++-\n diff.c | 25 +++++++++++++++++++++++--\n diff.h | 1 +\n 3 files changed, 28 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 34f0145..e0429b3 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -48,11 +48,14 @@ endif::git-format-patch[]\n --patience::\n \tGenerate a diff using the \"patience diff\" algorithm.\n \n---stat[=[,]]::\n+--stat[=[,[,]]]::\n \tGenerate a diffstat. You can override the default\n \toutput width for 80-column terminal by `--stat=`.\n \tThe width of the filename part can be controlled by\n \tgiving another width to it separated by a comma.\n+\tBy giving a third parameter ``, you can limit the\n+\toutput to the first and last `` lines, separated by\n+\t`...`.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\ndiff --git a/diff.c b/diff.c\nindex feced34..9ccba1e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1244,7 +1244,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tint i, len, add, del, adds = 0, dels = 0;\n \tuintmax_t max_change = 0, max_len = 0;\n \tint total_files = data->nr;\n-\tint width, name_width;\n+\tint width, name_width, lines;\n \tconst char *reset, *add_c, *del_c;\n \tconst char *line_prefix = \"\";\n \tstruct strbuf *msg = NULL;\n@@ -1259,6 +1259,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \n \twidth = options->stat_width ? options->stat_width : 80;\n \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n+\tlines = options->stat_lines;\n \n \t/* Sanity: give at least 5 columns to the graph,\n \t * but leave at least 10 columns for the name.\n@@ -1303,6 +1304,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\twidth = max_change;\n \n \tfor (i = 0; i < data->nr; i++) {\n+\t\tif (lines && i >= lines && i < data->nr-lines) {\n+\t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n+\t\t\ti = data->nr-lines-1;\n+\t\t\tlines = 0; /* no need to recheck */\n+\t\t\tcontinue;\n+\t\t}\n \t\tconst char *prefix = \"\";\n \t\tchar *name = data->files[i]->print_name;\n \t\tuintmax_t added = data->files[i]->added;\n@@ -3105,6 +3112,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \tchar *end;\n \tint width = options->stat_width;\n \tint name_width = options->stat_name_width;\n+\tint lines = options->stat_lines;\n \tint argcount = 1;\n \n \targ += strlen(\"--stat\");\n@@ -3132,12 +3140,24 @@ static int stat_opt(struct diff_options *options, const char **av)\n \t\t\t\tname_width = strtoul(av[1], &end, 10);\n \t\t\t\targcount = 2;\n \t\t\t}\n+\t\t} else if (!prefixcmp(arg, \"-lines\")) {\n+\t\t\targ += strlen(\"-lines\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\tlines = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-lines' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\tlines = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n \t\t}\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n \t\tif (*end == ',')\n \t\t\tname_width = strtoul(end+1, &end, 10);\n+\t\tif (*end == ',')\n+\t\t\tlines = strtoul(end+1, &end, 10);\n \t}\n \n \t/* Important! This checks all the error cases! */\n@@ -3146,6 +3166,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \toptions->stat_name_width = name_width;\n \toptions->stat_width = width;\n+\toptions->stat_lines = lines;\n \treturn argcount;\n }\n \n@@ -3191,7 +3212,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n \telse if (!prefixcmp(arg, \"--stat\"))\n-\t\t/* --stat, --stat-width, or --stat-name-width */\n+\t\t/* --stat, --stat-width, --stat-name-width, or --stat-lines */\n \t\treturn stat_opt(options, av);\n \n \t/* renames options */\ndiff --git a/diff.h b/diff.h\nindex d83e53e..98e9aed 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -124,6 +124,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tint stat_lines;\n \tconst char *word_regex;\n \tenum diff_words_type word_diff;\n \n-- \n1.7.5.250.g4493b\n"},{"id":"166736","messageId":"554098870eea307ea8f16e217f05413b5fe8fb5a.1304089050.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"5f16db0f3730be70ff522e63fbd491dc910c34d0.1304089050.git.git@drmicha.warpmail.net","subject":"[PATCH 2/2] diff-options.txt: describe --stat-{width,name-width,lines}","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-29T14:57:46Z","receivedAt":"2011-04-29T14:57:46Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/diff-options.txt | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex e0429b3..ed8dbef 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -56,6 +56,9 @@ endif::git-format-patch[]\n \tBy giving a third parameter ``, you can limit the\n \toutput to the first and last `` lines, separated by\n \t`...`.\n++\n+These parameters can also be set individually with `--stat-width=`,\n+`--stat-name-width=` and `--stat-lines=`.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\n-- \n1.7.5.250.g4493b\n"},{"id":"166739","messageId":"4DBAD56C.8020804@drmicha.warpmail.net","threadId":"27218","inReplyTo":"5f16db0f3730be70ff522e63fbd491dc910c34d0.1304089050.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-29T15:12:44Z","receivedAt":"2011-04-29T15:12:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Michael J Gruber venit, vidit, dixit 29.04.2011 16:57:\n> Often one is interested in the full --stat output only for commits which\n> change a few files, but not others, because larger restructuring gives a\n> --stat which fills a few screens.\n> \n> Introduce a new option --stat-lines= which limits the --stat output\n> to the first and last lines, separated by a \"...\" line. It can\n> also be given as the third parameter in\n> --stat=,,.\n> \n> Also, the unstuck form is supported analogous to the other two stat\n> parameters.\n> \n> Signed-off-by: Michael J Gruber \n> ---\n> I would even consider a default of 10 (i.e. show a 20 line stat in full,\n> abbreviate larger ones) to be sensible but have refrained from such a\n> behaviour change.\n> \n> We have hardcoded defaults for width (80) and name-width (50), so having\n> one for lines should be okay also. Can I has tis wiz default? ;)\n> ---\n> Documentation/diff-options.txt | 5 ++++-\n> diff.c | 25 +++++++++++++++++++++++--\n> diff.h | 1 +\n> 3 files changed, 28 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 34f0145..e0429b3 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -48,11 +48,14 @@ endif::git-format-patch[]\n> --patience::\n> \tGenerate a diff using the \"patience diff\" algorithm.\n> \n> ---stat[=[,]]::\n> +--stat[=[,[,]]]::\n> \tGenerate a diffstat. You can override the default\n> \toutput width for 80-column terminal by `--stat=`.\n> \tThe width of the filename part can be controlled by\n> \tgiving another width to it separated by a comma.\n> +\tBy giving a third parameter ``, you can limit the\n> +\toutput to the first and last `` lines, separated by\n> +\t`...`.\n> \n> --numstat::\n> \tSimilar to `\\--stat`, but shows number of added and\n> diff --git a/diff.c b/diff.c\n> index feced34..9ccba1e 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1244,7 +1244,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \tint i, len, add, del, adds = 0, dels = 0;\n> \tuintmax_t max_change = 0, max_len = 0;\n> \tint total_files = data->nr;\n> -\tint width, name_width;\n> +\tint width, name_width, lines;\n> \tconst char *reset, *add_c, *del_c;\n> \tconst char *line_prefix = \"\";\n> \tstruct strbuf *msg = NULL;\n> @@ -1259,6 +1259,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \n> \twidth = options->stat_width ? options->stat_width : 80;\n> \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n> +\tlines = options->stat_lines;\n> \n> \t/* Sanity: give at least 5 columns to the graph,\n> \t * but leave at least 10 columns for the name.\n> @@ -1303,6 +1304,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\twidth = max_change;\n> \n> \tfor (i = 0; i < data->nr; i++) {\n> +\t\tif (lines && i >= lines && i < data->nr-lines) {\n> +\t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n> +\t\t\ti = data->nr-lines-1;\n> +\t\t\tlines = 0; /* no need to recheck */\n> +\t\t\tcontinue;\n> +\t\t}\n\nOne could/should do the same limitting for the earlier loop which\ncomputes the maximal line width (since the largest file names may be\nskipped now). I'll leave this for a v2 which I'm sure will be necessary :)\n\nMichael\n"},{"id":"166742","messageId":"7voc3pyqy4.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"5f16db0f3730be70ff522e63fbd491dc910c34d0.1304089050.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-29T16:15:15Z","receivedAt":"2011-04-29T16:15:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> I would even consider a default of 10 (i.e. show a 20 line stat in full,\n> abbreviate larger ones) to be sensible but have refrained from such a\n> behaviour change.\n>\n> We have hardcoded defaults for width (80) and name-width (50), so having\n> one for lines should be okay also. Can I has tis wiz default? ;)\n\nThe terminal has fixed width to be apportioned but has an infinite height\nwith scrolling, so that comparison is somewhat bogus.\n\nThis should be called \"count\", not \"lines\". Anticipate a future where we\nintroduce a variant of --stat (in a way similar to how --numstat is a\nvariant of --stat) that displays its result with two lines per entry.\n\nAlso getting (N+1) \"lines\" output for specifying N feels very unnatural.\n"},{"id":"166810","messageId":"1304238428.8704.1446945897@webmail.messagingengine.com","threadId":"27218","inReplyTo":"7voc3pyqy4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-01T08:27:08Z","receivedAt":"2011-05-01T08:27:08Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"On Fri, 29 Apr 2011 09:15 -0700, \"Junio C Hamano\" \nwrote:\n> Michael J Gruber writes:\n> \n> > I would even consider a default of 10 (i.e. show a 20 line stat in full,\n> > abbreviate larger ones) to be sensible but have refrained from such a\n> > behaviour change.\n> >\n> > We have hardcoded defaults for width (80) and name-width (50), so having\n> > one for lines should be okay also. Can I has tis wiz default? ;)\n> \n> The terminal has fixed width to be apportioned but has an infinite height\n> with scrolling, so that comparison is somewhat bogus.\n> \n> This should be called \"count\", not \"lines\". Anticipate a future where we\n> introduce a variant of --stat (in a way similar to how --numstat is a\n> variant of --stat) that displays its result with two lines per entry.\n> \n> Also getting (N+1) \"lines\" output for specifying N feels very unnatural.\n> \nIt's actually 2*(N+1): The first and last N, plus the \"...\" line.\n\nWith \"count\" I would expect to see the first N lines/items. That's even\nsimpler to do. Do you think that makes more sense then head+tail?\n\nMichael\n"},{"id":"166818","messageId":"7vmxj6s22r.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"1304238428.8704.1446945897@webmail.messagingengine.com","subject":"Re: [PATCH 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-01T18:33:32Z","receivedAt":"2011-05-01T18:33:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael J Gruber\" writes:\n\n> On Fri, 29 Apr 2011 09:15 -0700, \"Junio C Hamano\" \n> wrote:\n>> Also getting (N+1) \"lines\" output for specifying N feels very unnatural.\n>> \n> It's actually 2*(N+1): The first and last N, plus the \"...\" line.\n\nYeah, that was what I meant, but I think my mistake itself demonstrates\nhow unintuitive \"N lines each from the top and the bottom end\" is.\n\nIt shouldn't be too difficult to divide the number given by the user by 2\nand adjust only one end when the given number is even, but I am not sure\nif that is worth doing, or if it is even a good idea to have \"...\" in the\nmiddle to begin with; it would make it harder to notice that the list is\nelided than having the marker at the end.\n\nBut what problem are you trying to solve?\n\nIf it is just you do not want to see a huge list, shouldn't that option\nalso be controlling how --summary is produced?\n\n> With \"count\" I would expect to see the first N lines/items. That's even\n> simpler to do. Do you think that makes more sense then head+tail?\n\n\"Hide noise and show only a few from the top and from the bottom\" makes\nsense when the items near the top and near the bottom are somehow more\nimportant than the others. Otherwise, \"keep only a few from the top\"\nshould work just as well.\n\nUsually, no single path is more important than the others, and even if\nthere were a path that is more important than the others, people tend to\narrange so that these more important paths come very early in the natural\nsort order (think \"00_README\"), so in that sense, items near the top could\nbe more important than items elsewhere, while items near the bottom are\nunlikely to have higher nor lower importance than the remainder.\n\nThe bottom of a sorted list could become a bit more important than others\nin a narrow corner case that we would likely not care: when your change is\nonly for paths whose names begin with 'a' through 'm', \"show only a few\nfrom the top and from the bottom\" would let you see that nothing between\n'n' and 'z' was touched. I do not think it is an important piece of\ninformation to convey, though.\n\nI designed merge.log (back then it was merge.summary) to show commits near\nthe top because the person reading the log would be better served by\nseeing up to what change the merge integrated the topic branch.\n\nA typical clean history of a topic branch tends to have necessary but\nuninteresting clean-up commits earier and the commits to complete the\nfeature near the top. In the context of reading the merge itself, you\ncare much more about why the merge was done (what the integration tree\nwanted out of the topic) than about how the topic progressed. Seeing the\nbottom (i.e. one commit above the fork point) may only be interesting for\nthe person who is making a merge of his own topic branch, but it is not\nvery useful for the integrator (the fork point itself may be a useful\nthing to know, though) nor for the reader of the history.\n\nBut the same \"bottom could be a bit more important than others\" argument\ncan apply here, and I wouldn't oppose to a patch to change \"merge.log\" to\nshow N-2 top entries, \"...\", and then a single bottom entry, when merging\nmore than N+1 commits to your branch.\n\nThat approach would probably apply equally well to your --stat-count and\ncovers the \"the bottom is slightly more important than others\" issue. It\nwould go like this:\n\n - Show the first N-2 items unconditionally;\n\n - If you have shown N-2 items already, keep counting without\n showing, and remember the last two items you saw;\n\n - At the end of the loop,\n\n - if you didn't discard any and the last ones you remember comes\n immediately after the first N-2 items, just show them;\n\n - if you skipped any, show \"... (%d items omitted)\" and then\n show the last item.\n"},{"id":"166933","messageId":"102e117f7a9f7fd6e652f23d5630db973e8e52b8.1304419588.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"7vmxj6s22r.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-03T10:46:38Z","receivedAt":"2011-05-03T10:46:38Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Often one is interested in the full --stat output only for commits which\nchange a few files, but not others, because larger restructuring gives a\n--stat which fills a few screens.\n\nIntroduce a new option --stat-count= which limits the --stat output\nto the first lines, followed by a \"...\" line. It can\nalso be given as the third parameter in\n--stat=,,.\n\nAlso, the unstuck form is supported analogous to the other two stat\nparameters.\n\nSigned-off-by: Michael J Gruber \n---\nI would even consider a default of 20 to be sensible but have refrained from\nsuch a behaviour change.\n\nWe have hardcoded defaults for width (80) and name-width (50), but - as Junio\npoints out - typical terminals are horizontally limited, not vertically.\n\nv2 uses the name \"count\" as suggested by Junio and limits to the first \nlines/items.\n\nIn the case with +1 items one may argue whether it makes more sense to\nignore the user wish and output all +1 lines, or lines (as\nrequested) plus the \"...\" line.\n\n(I saw the suggestion about N-2...2 just now. Would work also, but I guess\nwe would do this in more cases then, as Junio indicated.)\n---\n Documentation/diff-options.txt | 5 ++++-\n diff.c | 37 +++++++++++++++++++++++++++++++++----\n diff.h | 1 +\n 3 files changed, 38 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 34f0145..000eae0 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -48,11 +48,14 @@ endif::git-format-patch[]\n --patience::\n \tGenerate a diff using the \"patience diff\" algorithm.\n \n---stat[=[,]]::\n+--stat[=[,[,]]]::\n \tGenerate a diffstat. You can override the default\n \toutput width for 80-column terminal by `--stat=`.\n \tThe width of the filename part can be controlled by\n \tgiving another width to it separated by a comma.\n+\tBy giving a third parameter ``, you can limit the\n+\toutput to the first `` lines, followed by\n+\t`...` if there are more.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\ndiff --git a/diff.c b/diff.c\nindex feced34..a6b0f0f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1244,7 +1244,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tint i, len, add, del, adds = 0, dels = 0;\n \tuintmax_t max_change = 0, max_len = 0;\n \tint total_files = data->nr;\n-\tint width, name_width;\n+\tint width, name_width, count;\n \tconst char *reset, *add_c, *del_c;\n \tconst char *line_prefix = \"\";\n \tstruct strbuf *msg = NULL;\n@@ -1259,6 +1259,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \n \twidth = options->stat_width ? options->stat_width : 80;\n \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n+\tcount = (options->stat_count && options->stat_count < data->nr)\n+\t\t\t\t\t? options->stat_count : data->nr;\n \n \t/* Sanity: give at least 5 columns to the graph,\n \t * but leave at least 10 columns for the name.\n@@ -1275,7 +1277,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tadd_c = diff_get_color_opt(options, DIFF_FILE_NEW);\n \tdel_c = diff_get_color_opt(options, DIFF_FILE_OLD);\n \n-\tfor (i = 0; i < data->nr; i++) {\n+\tfor (i = 0; i < count; i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t change = file->added + file->deleted;\n \t\tfill_print_name(file);\n@@ -1302,7 +1304,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \telse\n \t\twidth = max_change;\n \n-\tfor (i = 0; i < data->nr; i++) {\n+\tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tchar *name = data->files[i]->print_name;\n \t\tuintmax_t added = data->files[i]->added;\n@@ -1369,6 +1371,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tshow_graph(options->file, '-', del, del_c, reset);\n \t\tfprintf(options->file, \"\\n\");\n \t}\n+\tif (count < data->nr)\n+\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n+\tfor (i = count; i < data->nr; i++) {\n+\t\tuintmax_t added = data->files[i]->added;\n+\t\tuintmax_t deleted = data->files[i]->deleted;\n+\t\tif (!data->files[i]->is_renamed &&\n+\t\t\t (added + deleted == 0)) {\n+\t\t\ttotal_files--;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tadds += added;\n+\t\tdels += deleted;\n+\t}\n \tfprintf(options->file, \"%s\", line_prefix);\n \tfprintf(options->file,\n \t \" %d files changed, %d insertions(+), %d deletions(-)\\n\",\n@@ -3105,6 +3120,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \tchar *end;\n \tint width = options->stat_width;\n \tint name_width = options->stat_name_width;\n+\tint count = options->stat_count;\n \tint argcount = 1;\n \n \targ += strlen(\"--stat\");\n@@ -3132,12 +3148,24 @@ static int stat_opt(struct diff_options *options, const char **av)\n \t\t\t\tname_width = strtoul(av[1], &end, 10);\n \t\t\t\targcount = 2;\n \t\t\t}\n+\t\t} else if (!prefixcmp(arg, \"-count\")) {\n+\t\t\targ += strlen(\"-count\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\tcount = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-count' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\tcount = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n \t\t}\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n \t\tif (*end == ',')\n \t\t\tname_width = strtoul(end+1, &end, 10);\n+\t\tif (*end == ',')\n+\t\t\tcount = strtoul(end+1, &end, 10);\n \t}\n \n \t/* Important! This checks all the error cases! */\n@@ -3146,6 +3174,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \toptions->stat_name_width = name_width;\n \toptions->stat_width = width;\n+\toptions->stat_count = count;\n \treturn argcount;\n }\n \n@@ -3191,7 +3220,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n \telse if (!prefixcmp(arg, \"--stat\"))\n-\t\t/* --stat, --stat-width, or --stat-name-width */\n+\t\t/* --stat, --stat-width, --stat-name-width, or --stat-count */\n \t\treturn stat_opt(options, av);\n \n \t/* renames options */\ndiff --git a/diff.h b/diff.h\nindex d83e53e..30ce9d8 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -124,6 +124,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tint stat_count;\n \tconst char *word_regex;\n \tenum diff_words_type word_diff;\n \n-- \n1.7.5.250.g4493b\n"},{"id":"166932","messageId":"ee73b430a470b85e18f7577f496da4e955810eae.1304419588.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"7vmxj6s22r.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 2/2] diff-options.txt: describe --stat-{width,name-width,count}","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-03T10:46:39Z","receivedAt":"2011-05-03T10:46:39Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/diff-options.txt | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 000eae0..f6c046a 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -56,6 +56,9 @@ endif::git-format-patch[]\n \tBy giving a third parameter ``, you can limit the\n \toutput to the first `` lines, followed by\n \t`...` if there are more.\n++\n+These parameters can also be set individually with `--stat-width=`,\n+`--stat-name-width=` and `--stat-count=`.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\n-- \n1.7.5.250.g4493b\n"},{"id":"166958","messageId":"7vfwoviptm.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"102e117f7a9f7fd6e652f23d5630db973e8e52b8.1304419588.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-03T18:47:33Z","receivedAt":"2011-05-03T18:47:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> In the case with +1 items one may argue whether it makes more sense to\n> ignore the user wish and output all +1 lines, or lines (as\n> requested) plus the \"...\" line.\n\nI think that is a must if we care about consistency. fmt-merge-msg should\nalready do this (I remember being careful about this particular case when\nI wrote its first version, but I do not know it has regressed as I do not\nremember writing tests for this boundary case---my bad ;-).\n\n> (I saw the suggestion about N-2...2 just now. Would work also, but I guess\n> we would do this in more cases then, as Junio indicated.)\n\nIt is not clear what you mean by N-2...2 but if you are referring to my\n\"first N-1 entries, dots and the last one, to make the total N+1 lines\nthat show N entries\", then yes I think it would make sense to do that also\nin fmt-merge-msg.c::shortlog() as well as here. But that would be a\nseparate topic.\n\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 34f0145..000eae0 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -48,11 +48,14 @@ endif::git-format-patch[]\n> --patience::\n> \tGenerate a diff using the \"patience diff\" algorithm.\n> \n> ---stat[=[,]]::\n> +--stat[=[,[,]]]::\n> \tGenerate a diffstat. You can override the default\n> \toutput width for 80-column terminal by `--stat=`.\n> \tThe width of the filename part can be controlled by\n> \tgiving another width to it separated by a comma.\n> +\tBy giving a third parameter ``, you can limit the\n> +\toutput to the first `` lines, followed by\n> +\t`...` if there are more.\n\nDoes an empty-string mean \"use default\" (currently \"no limit\")?\nThis matters when we teach a new parameter to --stat and make the above:\n\n\t--stat=[=[,[,[,]]]]\n\n> +\tfor (i = count; i < data->nr; i++) {\n> +\t\tuintmax_t added = data->files[i]->added;\n> +\t\tuintmax_t deleted = data->files[i]->deleted;\n> +\t\tif (!data->files[i]->is_renamed &&\n> +\t\t\t (added + deleted == 0)) {\n> +\t\t\ttotal_files--;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\t\tadds += added;\n> +\t\tdels += deleted;\n> +\t}\n> \tfprintf(options->file, \"%s\", line_prefix);\n> \tfprintf(options->file,\n> \t \" %d files changed, %d insertions(+), %d deletions(-)\\n\",\n\nThis is culling the output of what is in struct diffstat that we have\nalready spent cycles to possibly fill thousands of entries. I first\nthought it may make sense to also tweak the loop in diff_flush() that runs\ndiff_flush_stat() to all filepairs to run it only on the first \n(and later the first and the last) filepairs, but we have to\nshow the short-stat summary at the end, so this cannot be avoided.\n\nWhat happens when I say \"diff --numstat --stat-count=4\"?\n\nShould it error out upon seeing a limit that is not infinite, or should it\nalso elide the lines in its output?\n"},{"id":"166990","messageId":"7vvcxrf5e7.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"102e117f7a9f7fd6e652f23d5630db973e8e52b8.1304419588.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-04T04:37:04Z","receivedAt":"2011-05-04T04:37:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> @@ -1302,7 +1304,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \telse\n> \t\twidth = max_change;\n> \n> -\tfor (i = 0; i < data->nr; i++) {\n> +\tfor (i = 0; i < count; i++) {\n> \t\tconst char *prefix = \"\";\n> \t\tchar *name = data->files[i]->print_name;\n> \t\tuintmax_t added = data->files[i]->added;\n\nThis first loop can omit a \"struct diffstat_file\" that is not a rename and\ndoes not add nor delete any lines (look for \"total_files--\"), but you do\nnot seem to compensate for it. If you have such a record in the earlier\npart of the result for whatever reason, you would end up showing fewer\nentries than what \"count\" indicates in this loop.\n"},{"id":"166997","messageId":"4DC0FD3D.9010004@drmicha.warpmail.net","threadId":"27218","inReplyTo":"7vfwoviptm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/2] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-04T07:16:13Z","receivedAt":"2011-05-04T07:16:13Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 03.05.2011 20:47:\n> Michael J Gruber writes:\n> \n>> In the case with +1 items one may argue whether it makes more sense to\n>> ignore the user wish and output all +1 lines, or lines (as\n>> requested) plus the \"...\" line.\n> \n> I think that is a must if we care about consistency. fmt-merge-msg should\n\nI assume you mean the part before the \"or\" by \"this\".\n\n> already do this (I remember being careful about this particular case when\n> I wrote its first version, but I do not know it has regressed as I do not\n> remember writing tests for this boundary case---my bad ;-).\n\nI'll recheck with fmt-merge-msg. Seems this series needs more work than\nI expected... and may take some time.\n\n>> (I saw the suggestion about N-2...2 just now. Would work also, but I guess\n>> we would do this in more cases then, as Junio indicated.)\n> \n> It is not clear what you mean by N-2...2 but if you are referring to my\n> \"first N-1 entries, dots and the last one, to make the total N+1 lines\n> that show N entries\", then yes I think it would make sense to do that also\n> in fmt-merge-msg.c::shortlog() as well as here. But that would be a\n> separate topic.\n\nYes, N-2, then the remaining 2 or \"...\" plus the last one. Sorry for the\nconfusion.\n\n>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n>> index 34f0145..000eae0 100644\n>> --- a/Documentation/diff-options.txt\n>> +++ b/Documentation/diff-options.txt\n>> @@ -48,11 +48,14 @@ endif::git-format-patch[]\n>> --patience::\n>> \tGenerate a diff using the \"patience diff\" algorithm.\n>> \n>> ---stat[=[,]]::\n>> +--stat[=[,[,]]]::\n>> \tGenerate a diffstat. You can override the default\n>> \toutput width for 80-column terminal by `--stat=`.\n>> \tThe width of the filename part can be controlled by\n>> \tgiving another width to it separated by a comma.\n>> +\tBy giving a third parameter ``, you can limit the\n>> +\toutput to the first `` lines, followed by\n>> +\t`...` if there are more.\n> \n> Does an empty-string mean \"use default\" (currently \"no limit\")?\n> This matters when we teach a new parameter to --stat and make the above:\n> \n> \t--stat=[=[,[,[,]]]]\n>\n\nYes, strtoul(\"\") is 0, and 0 denotes default, which is unlimited. By\ndesign, but maybe I should mention it somewhere, probably as part of\n2/2, because that same effect is true and undocumented for the 1st 2\nslots, i.e. --stat=\",,5\" sets the count argument to 5 and the others to\ndefault.\n\n>> +\tfor (i = count; i < data->nr; i++) {\n>> +\t\tuintmax_t added = data->files[i]->added;\n>> +\t\tuintmax_t deleted = data->files[i]->deleted;\n>> +\t\tif (!data->files[i]->is_renamed &&\n>> +\t\t\t (added + deleted == 0)) {\n>> +\t\t\ttotal_files--;\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\t\tadds += added;\n>> +\t\tdels += deleted;\n>> +\t}\n>> \tfprintf(options->file, \"%s\", line_prefix);\n>> \tfprintf(options->file,\n>> \t \" %d files changed, %d insertions(+), %d deletions(-)\\n\",\n> \n> This is culling the output of what is in struct diffstat that we have\n> already spent cycles to possibly fill thousands of entries. I first\n> thought it may make sense to also tweak the loop in diff_flush() that runs\n> diff_flush_stat() to all filepairs to run it only on the first \n> (and later the first and the last) filepairs, but we have to\n> show the short-stat summary at the end, so this cannot be avoided.\n\nYeah, in my 0th version that stat was for the shown lines only...\n\n> What happens when I say \"diff --numstat --stat-count=4\"?\n> \n> Should it error out upon seeing a limit that is not infinite, or should it\n> also elide the lines in its output?\n\nNone of the --stat-* options affects --numstat, again by design. All of\nwidth, name-width and count would make sense for --numstat (and\n--dirstat) also, but that would require some restructuring.\n\n> Michael J Gruber writes:\n> \n>> > @@ -1302,7 +1304,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> > \telse\n>> > \t\twidth = max_change;\n>> > \n>> > -\tfor (i = 0; i < data->nr; i++) {\n>> > +\tfor (i = 0; i < count; i++) {\n>> > \t\tconst char *prefix = \"\";\n>> > \t\tchar *name = data->files[i]->print_name;\n>> > \t\tuintmax_t added = data->files[i]->added;\n> This first loop can omit a \"struct diffstat_file\" that is not a rename and\n> does not add nor delete any lines (look for \"total_files--\"), but you do\n> not seem to compensate for it. If you have such a record in the earlier\n> part of the result for whatever reason, you would end up showing fewer\n> entries than what \"count\" indicates in this loop.\n\nUh, sorry and thanks for noticing. I wasn't aware that we may skip a\npair completely.\n\nI wanted to make the design optimal in the sense that I introduce as few\nadditional conditionals within the two loops there. I guess I'll have to\nsacrifice that in the first loop.\n\nMichael\n"},{"id":"168863","messageId":"cover.1306499600.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"4DC0FD3D.9010004@drmicha.warpmail.net","subject":"[PATCHv3 0/3] diff.c: --stat-count=","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-27T12:36:39Z","receivedAt":"2011-05-27T12:36:39Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"So, this grew into a little series because of the issue in 1/3. The fact\nthat we looked up the name for a diff item even when we don't display it\nwas never great but started being bad when we started not to fill in\nunnecessary names... With that segv out of the way, here's v3 of the series.\n\nCompared to v2, I hope I have learned to count.\n\nMichael J Gruber (3):\n diff.c: omit hidden entries from namelen calculation with --stat\n diff: introduce --stat-lines to limit the stat lines\n diff-options.txt: describe --stat-{width,name-width,count}\n\n Documentation/diff-options.txt | 8 +++++-\n diff.c | 52 +++++++++++++++++++++++++++++++++-------\n diff.h | 1 +\n 3 files changed, 51 insertions(+), 10 deletions(-)\n\n-- \n1.7.5.2.657.g62c2\n"},{"id":"168864","messageId":"b47e2f0865bac1ad0e7b777ce9f27493292c502c.1306499600.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"4DC0FD3D.9010004@drmicha.warpmail.net","subject":"[PATCHv3 1/3] diff.c: omit hidden entries from namelen calculation with --stat","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-27T12:36:40Z","receivedAt":"2011-05-27T12:36:40Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, --stat calculates the longest name from all items but then\ndrops some (mode changes) from the output later on.\n\nInstead, drop them from the namelen generation and calculation.\n\nSigned-off-by: Michael J Gruber \n---\nThis optimizes (tightens) the display potentially, but we never had tests\nwhich are sensitive to that.\n---\n diff.c | 14 +++++++++-----\n 1 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex feced34..4541939 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1278,6 +1278,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < data->nr; i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t change = file->added + file->deleted;\n+\t\tif (!data->files[i]->is_renamed &&\n+\t\t\t (change == 0)) {\n+\t\t\tcontinue;\n+\t\t}\n \t\tfill_print_name(file);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n@@ -1309,6 +1313,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tuintmax_t deleted = data->files[i]->deleted;\n \t\tint name_len;\n \n+\t\tif (!data->files[i]->is_renamed &&\n+\t\t\t (added + deleted == 0)) {\n+\t\t\ttotal_files--;\n+\t\t\tcontinue;\n+\t\t}\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1343,11 +1352,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tfprintf(options->file, \" Unmerged\\n\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (!data->files[i]->is_renamed &&\n-\t\t\t (added + deleted == 0)) {\n-\t\t\ttotal_files--;\n-\t\t\tcontinue;\n-\t\t}\n \n \t\t/*\n \t\t * scale the add/delete\n-- \n1.7.5.2.657.g62c2\n"},{"id":"168865","messageId":"5da631c64438ec3f669f0c2b7456bcfbc371e2f5.1306499600.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"4DC0FD3D.9010004@drmicha.warpmail.net","subject":"[PATCHv3 2/3] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-27T12:36:41Z","receivedAt":"2011-05-27T12:36:41Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Often one is interested in the full --stat output only for commits which\nchange a few files, but not others, because larger restructuring gives a\n--stat which fills a few screens.\n\nIntroduce a new option --stat-count= which limits the --stat output\nto the first lines, followed by a \"...\" line. It can\nalso be given as the third parameter in\n--stat=,,.\n\nAlso, the unstuck form is supported analogous to the other two stat\nparameters.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/diff-options.txt | 5 ++++-\n diff.c | 38 ++++++++++++++++++++++++++++++++++----\n diff.h | 1 +\n 3 files changed, 39 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 34f0145..000eae0 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -48,11 +48,14 @@ endif::git-format-patch[]\n --patience::\n \tGenerate a diff using the \"patience diff\" algorithm.\n \n---stat[=[,]]::\n+--stat[=[,[,]]]::\n \tGenerate a diffstat. You can override the default\n \toutput width for 80-column terminal by `--stat=`.\n \tThe width of the filename part can be controlled by\n \tgiving another width to it separated by a comma.\n+\tBy giving a third parameter ``, you can limit the\n+\toutput to the first `` lines, followed by\n+\t`...` if there are more.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\ndiff --git a/diff.c b/diff.c\nindex 4541939..6a6cbca 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1244,7 +1244,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tint i, len, add, del, adds = 0, dels = 0;\n \tuintmax_t max_change = 0, max_len = 0;\n \tint total_files = data->nr;\n-\tint width, name_width;\n+\tint width, name_width, count;\n \tconst char *reset, *add_c, *del_c;\n \tconst char *line_prefix = \"\";\n \tstruct strbuf *msg = NULL;\n@@ -1259,6 +1259,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \n \twidth = options->stat_width ? options->stat_width : 80;\n \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n+\tcount = options->stat_count ? options->stat_count : data->nr;\n \n \t/* Sanity: give at least 5 columns to the graph,\n \t * but leave at least 10 columns for the name.\n@@ -1275,11 +1276,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tadd_c = diff_get_color_opt(options, DIFF_FILE_NEW);\n \tdel_c = diff_get_color_opt(options, DIFF_FILE_OLD);\n \n-\tfor (i = 0; i < data->nr; i++) {\n+\tfor (i = 0; (i < count) && (i < data->nr); i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t change = file->added + file->deleted;\n \t\tif (!data->files[i]->is_renamed &&\n \t\t\t (change == 0)) {\n+\t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n \t\tfill_print_name(file);\n@@ -1292,6 +1294,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (max_change < change)\n \t\t\tmax_change = change;\n \t}\n+\tcount = i; /* min(count, data->nr) */\n \n \t/* Compute the width of the graph part;\n \t * 10 is for one blank at the beginning of the line plus\n@@ -1306,7 +1309,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \telse\n \t\twidth = max_change;\n \n-\tfor (i = 0; i < data->nr; i++) {\n+\tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tchar *name = data->files[i]->print_name;\n \t\tuintmax_t added = data->files[i]->added;\n@@ -1373,6 +1376,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tshow_graph(options->file, '-', del, del_c, reset);\n \t\tfprintf(options->file, \"\\n\");\n \t}\n+\tif (count < data->nr)\n+\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n+\tfor (i = count; i < data->nr; i++) {\n+\t\tuintmax_t added = data->files[i]->added;\n+\t\tuintmax_t deleted = data->files[i]->deleted;\n+\t\tif (!data->files[i]->is_renamed &&\n+\t\t\t (added + deleted == 0)) {\n+\t\t\ttotal_files--;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tadds += added;\n+\t\tdels += deleted;\n+\t}\n \tfprintf(options->file, \"%s\", line_prefix);\n \tfprintf(options->file,\n \t \" %d files changed, %d insertions(+), %d deletions(-)\\n\",\n@@ -3109,6 +3125,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \tchar *end;\n \tint width = options->stat_width;\n \tint name_width = options->stat_name_width;\n+\tint count = options->stat_count;\n \tint argcount = 1;\n \n \targ += strlen(\"--stat\");\n@@ -3136,12 +3153,24 @@ static int stat_opt(struct diff_options *options, const char **av)\n \t\t\t\tname_width = strtoul(av[1], &end, 10);\n \t\t\t\targcount = 2;\n \t\t\t}\n+\t\t} else if (!prefixcmp(arg, \"-count\")) {\n+\t\t\targ += strlen(\"-count\");\n+\t\t\tif (*arg == '=')\n+\t\t\t\tcount = strtoul(arg + 1, &end, 10);\n+\t\t\telse if (!*arg && !av[1])\n+\t\t\t\tdie(\"Option '--stat-count' requires a value\");\n+\t\t\telse if (!*arg) {\n+\t\t\t\tcount = strtoul(av[1], &end, 10);\n+\t\t\t\targcount = 2;\n+\t\t\t}\n \t\t}\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n \t\tif (*end == ',')\n \t\t\tname_width = strtoul(end+1, &end, 10);\n+\t\tif (*end == ',')\n+\t\t\tcount = strtoul(end+1, &end, 10);\n \t}\n \n \t/* Important! This checks all the error cases! */\n@@ -3150,6 +3179,7 @@ static int stat_opt(struct diff_options *options, const char **av)\n \toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \toptions->stat_name_width = name_width;\n \toptions->stat_width = width;\n+\toptions->stat_count = count;\n \treturn argcount;\n }\n \n@@ -3195,7 +3225,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"-s\"))\n \t\toptions->output_format |= DIFF_FORMAT_NO_OUTPUT;\n \telse if (!prefixcmp(arg, \"--stat\"))\n-\t\t/* --stat, --stat-width, or --stat-name-width */\n+\t\t/* --stat, --stat-width, --stat-name-width, or --stat-count */\n \t\treturn stat_opt(options, av);\n \n \t/* renames options */\ndiff --git a/diff.h b/diff.h\nindex d83e53e..30ce9d8 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -124,6 +124,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tint stat_count;\n \tconst char *word_regex;\n \tenum diff_words_type word_diff;\n \n-- \n1.7.5.2.657.g62c2\n"},{"id":"168866","messageId":"116faa98951a64770efeb44d54adc4b0da359f57.1306499600.git.git@drmicha.warpmail.net","threadId":"27218","inReplyTo":"4DC0FD3D.9010004@drmicha.warpmail.net","subject":"[PATCHv3 3/3] diff-options.txt: describe --stat-{width,name-width,count}","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-27T12:36:42Z","receivedAt":"2011-05-27T12:36:42Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/diff-options.txt | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 000eae0..f6c046a 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -56,6 +56,9 @@ endif::git-format-patch[]\n \tBy giving a third parameter ``, you can limit the\n \toutput to the first `` lines, followed by\n \t`...` if there are more.\n++\n+These parameters can also be set individually with `--stat-width=`,\n+`--stat-name-width=` and `--stat-count=`.\n \n --numstat::\n \tSimilar to `\\--stat`, but shows number of added and\n-- \n1.7.5.2.657.g62c2\n"},{"id":"168884","messageId":"7vvcwwrqd7.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"b47e2f0865bac1ad0e7b777ce9f27493292c502c.1306499600.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3 1/3] diff.c: omit hidden entries from namelen calculation with --stat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-27T17:43:32Z","receivedAt":"2011-05-27T17:43:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, --stat calculates the longest name from all items but then\n> drops some (mode changes) from the output later on.\n>\n> Instead, drop them from the namelen generation and calculation.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> This optimizes (tightens) the display potentially, but we never had tests\n> which are sensitive to that.\n\nMore importantly I think this is a good first step in the right direction\nto reduce the duplicated code that need to implement the same logic.\n\nHowever, you missed another instance in show_shortstats() no?\n\nI am wondering if it would make it easier to both read and maintain if you\nadd a boolean field \"changed\" to \"struct diffstat_file\", and set it at the\nend of builtin_diffstat(), and have these places all check that field.\n\nA possible alternative is not to put such an unchanged entry in the struct\ndiffstat_t in the first place so that nobody has to worry about it. Right\nnow, show_numstat() does show 0/0 entries (i.e. only mode change), while\nshortstat and normal stat do not, but I somehow have a feeling that this\ndifference is not by design but by accident. Besides, --numstat that only\nsays 0/0 is not useful in practice without --summary.\n\n $ edit diff.c\n $ chmod +x Makefile\n $ git diff --stat\n diff.c | 26 ++++++++++++++------------\n $ git diff --numstat\n 0 0 Makefile\n 14 12 diff.c\n $ git diff --numstat --summary\n 0 0 Makefile\n 14 12 diff.c\n mode change 100644 => 100755 Makefile\n\nGetting rid of an unchanged entry from the diffstat array upfront will\nchange the behaviour of numstat, but I suspect that it change things in a\nbetter way, making the result consistent with the textual version.\n\nThe patch below, diff.c shows that approach, while ndiff.c shows the extra\nboolean \"changed\" approach.\n\nWhat do you think?\n\n diff.c | 7 +++++++\n diff.c => ndiff.c | 26 ++++++++++++++------------\n 2 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8f4815b..e2ee70a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2267,6 +2267,13 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\t\t &xpp, &xecfg);\n \t}\n \n+\tif (!data->is_renamed && !data->added && !data->deleted) {\n+\t\tdiffstat->nr--;\n+\t\tfree(data->name);\n+\t\tfree(data->from_name);\n+\t\tfree(data);\n+\t}\n+\n \tdiff_free_filespec_data(one);\n \tdiff_free_filespec_data(two);\n }\ndiff --git a/diff.c b/ndiff.c\nsimilarity index 99%\ncopy from diff.c\ncopy to ndiff.c\nindex 8f4815b..1620053 100644\n--- a/diff.c\n+++ b/ndiff.c\n@@ -1222,6 +1222,7 @@ struct diffstat_t {\n \t\tunsigned is_unmerged:1;\n \t\tunsigned is_binary:1;\n \t\tunsigned is_renamed:1;\n+\t\tunsigned changed:1;\n \t\tuintmax_t added, deleted;\n \t} **files;\n };\n@@ -1350,6 +1351,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < data->nr; i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t change = file->added + file->deleted;\n+\n+\t\tif (!file->changed)\n+\t\t\tcontinue;\n \t\tfill_print_name(file);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n@@ -1381,6 +1385,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tuintmax_t deleted = data->files[i]->deleted;\n \t\tint name_len;\n \n+\t\tif (!data->files[i]->changed) {\n+\t\t\ttotal_files--;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1415,11 +1424,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tfprintf(options->file, \" Unmerged\\n\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (!data->files[i]->is_renamed &&\n-\t\t\t (added + deleted == 0)) {\n-\t\t\ttotal_files--;\n-\t\t\tcontinue;\n-\t\t}\n \n \t\t/*\n \t\t * scale the add/delete\n@@ -1457,15 +1461,12 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option\n \tfor (i = 0; i < data->nr; i++) {\n \t\tif (!data->files[i]->is_binary &&\n \t\t !data->files[i]->is_unmerged) {\n-\t\t\tint added = data->files[i]->added;\n-\t\t\tint deleted= data->files[i]->deleted;\n-\t\t\tif (!data->files[i]->is_renamed &&\n-\t\t\t (added + deleted == 0)) {\n+\t\t\tif (!data->files[i]->changed) {\n \t\t\t\ttotal_files--;\n-\t\t\t} else {\n-\t\t\t\tadds += added;\n-\t\t\t\tdels += deleted;\n+\t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tadds += data->files[i]->added;\n+\t\t\tdels += data->files[i]->deleted;\n \t\t}\n \t}\n \tif (options->output_prefix) {\n@@ -2266,6 +2267,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\txdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,\n \t\t\t &xpp, &xecfg);\n \t}\n+\tdata->changed = data->is_renamed || data->added || data->deleted;\n \n \tdiff_free_filespec_data(one);\n \tdiff_free_filespec_data(two);\n"},{"id":"168898","messageId":"4DE00755.1020404@drmicha.warpmail.net","threadId":"27218","inReplyTo":"7vvcwwrqd7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 1/3] diff.c: omit hidden entries from namelen calculation with --stat","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-27T20:19:33Z","receivedAt":"2011-05-27T20:19:33Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 27.05.2011 19:43:\n> Michael J Gruber writes:\n> \n>> Currently, --stat calculates the longest name from all items but then\n>> drops some (mode changes) from the output later on.\n>>\n>> Instead, drop them from the namelen generation and calculation.\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n>> This optimizes (tightens) the display potentially, but we never had tests\n>> which are sensitive to that.\n> \n> More importantly I think this is a good first step in the right direction\n> to reduce the duplicated code that need to implement the same logic.\n> \n> However, you missed another instance in show_shortstats() no?\n\nI'm sorry I don't understand this remark. For all different formats, the\nrespective functions show_stat(), show_shortstats() etc. do their own\nformatting, and I did not change this at all. So, what did I miss?\n\nAlso, show_shortstats() does not need the names, so what similar change\nshould one have made there?\n\n> I am wondering if it would make it easier to both read and maintain if you\n> add a boolean field \"changed\" to \"struct diffstat_file\", and set it at the\n> end of builtin_diffstat(), and have these places all check that field.\n\nSounds fine.\n\n> A possible alternative is not to put such an unchanged entry in the struct\n> diffstat_t in the first place so that nobody has to worry about it. Right\n> now, show_numstat() does show 0/0 entries (i.e. only mode change), while\n> shortstat and normal stat do not, but I somehow have a feeling that this\n> difference is not by design but by accident. Besides, --numstat that only\n> says 0/0 is not useful in practice without --summary.\n> \n> $ edit diff.c\n> $ chmod +x Makefile\n> $ git diff --stat\n> diff.c | 26 ++++++++++++++------------\n> $ git diff --numstat\n> 0 0 Makefile\n> 14 12 diff.c\n> $ git diff --numstat --summary\n> 0 0 Makefile\n> 14 12 diff.c\n> mode change 100644 => 100755 Makefile\n> \n> Getting rid of an unchanged entry from the diffstat array upfront will\n> change the behaviour of numstat, but I suspect that it change things in a\n> better way, making the result consistent with the textual version.\n\nThat sounds good to me.\n\n> The patch below, diff.c shows that approach, while ndiff.c shows the extra\n> boolean \"changed\" approach.\n> \n> What do you think?\n> \n> diff.c | 7 +++++++\n> diff.c => ndiff.c | 26 ++++++++++++++------------\n> 2 files changed, 21 insertions(+), 12 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 8f4815b..e2ee70a 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2267,6 +2267,13 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n> \t\t\t &xpp, &xecfg);\n> \t}\n> \n> +\tif (!data->is_renamed && !data->added && !data->deleted) {\n> +\t\tdiffstat->nr--;\n> +\t\tfree(data->name);\n> +\t\tfree(data->from_name);\n> +\t\tfree(data);\n> +\t}\n> +\n\nI don't know that codepath very well, but hasn't that diffstat been\nadded earlier in builtin_diffstat() already? But maybe the\ndiffstat->nr-- takes care of that.\n\nAlso, I have no idea how builtin_diffstat() influences show_stat() which\nmy patch was about, sorry.\n\n> \tdiff_free_filespec_data(one);\n> \tdiff_free_filespec_data(two);\n> }\n> diff --git a/diff.c b/ndiff.c\n> similarity index 99%\n> copy from diff.c\n> copy to ndiff.c\n> index 8f4815b..1620053 100644\n> --- a/diff.c\n> +++ b/ndiff.c\n> @@ -1222,6 +1222,7 @@ struct diffstat_t {\n> \t\tunsigned is_unmerged:1;\n> \t\tunsigned is_binary:1;\n> \t\tunsigned is_renamed:1;\n> +\t\tunsigned changed:1;\n> \t\tuintmax_t added, deleted;\n> \t} **files;\n> };\n> @@ -1350,6 +1351,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \tfor (i = 0; i < data->nr; i++) {\n> \t\tstruct diffstat_file *file = data->files[i];\n> \t\tuintmax_t change = file->added + file->deleted;\n> +\n> +\t\tif (!file->changed)\n> +\t\t\tcontinue;\n> \t\tfill_print_name(file);\n> \t\tlen = strlen(file->print_name);\n> \t\tif (max_len < len)\n> @@ -1381,6 +1385,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\tuintmax_t deleted = data->files[i]->deleted;\n> \t\tint name_len;\n> \n> +\t\tif (!data->files[i]->changed) {\n> +\t\t\ttotal_files--;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\t/*\n> \t\t * \"scale\" the filename\n> \t\t */\n> @@ -1415,11 +1424,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t\tfprintf(options->file, \" Unmerged\\n\");\n> \t\t\tcontinue;\n> \t\t}\n> -\t\telse if (!data->files[i]->is_renamed &&\n> -\t\t\t (added + deleted == 0)) {\n> -\t\t\ttotal_files--;\n> -\t\t\tcontinue;\n> -\t\t}\n\nor do this dance only here (with ...->changed) instead of in the first\nloop, because in 2/3 (when limited by stat_count), the second loop would\nbe done all the way up to nr (as 2 separate loops), but the first one\nonly up to count.\n\n> \n> \t\t/*\n> \t\t * scale the add/delete\n> @@ -1457,15 +1461,12 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option\n> \tfor (i = 0; i < data->nr; i++) {\n> \t\tif (!data->files[i]->is_binary &&\n> \t\t !data->files[i]->is_unmerged) {\n> -\t\t\tint added = data->files[i]->added;\n> -\t\t\tint deleted= data->files[i]->deleted;\n> -\t\t\tif (!data->files[i]->is_renamed &&\n> -\t\t\t (added + deleted == 0)) {\n> +\t\t\tif (!data->files[i]->changed) {\n> \t\t\t\ttotal_files--;\n> -\t\t\t} else {\n> -\t\t\t\tadds += added;\n> -\t\t\t\tdels += deleted;\n> +\t\t\t\tcontinue;\n> \t\t\t}\n> +\t\t\tadds += data->files[i]->added;\n> +\t\t\tdels += data->files[i]->deleted;\n> \t\t}\n> \t}\n> \tif (options->output_prefix) {\n> @@ -2266,6 +2267,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n> \t\txdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,\n> \t\t\t &xpp, &xecfg);\n> \t}\n> +\tdata->changed = data->is_renamed || data->added || data->deleted;\n> \n> \tdiff_free_filespec_data(one);\n> \tdiff_free_filespec_data(two);\n\nDoes show_stats() get the data as modified by builtin_diffstat()? Then\nthis is fine.\n\nAgain, I'm sorry I'm not seeing through the call chain here.\n\nMichael\n"},{"id":"168901","messageId":"7v39jzsqxt.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"4DE00755.1020404@drmicha.warpmail.net","subject":"Re: [PATCHv3 1/3] diff.c: omit hidden entries from namelen calculation with --stat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-27T22:45:50Z","receivedAt":"2011-05-27T22:45:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Again, I'm sorry I'm not seeing through the call chain here.\n\ndiff_flush() notices it needs to produce textual patch and count added or\ndeleted lines here:\n\n\tif (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT...\n\t\tstruct diffstat_t diffstat;\n\n\t\tmemset(&diffstat, 0, sizeof(struct diffstat_t));\n\t\tfor (i = 0; i < q->nr; i++) {\n\t\t\tstruct diff_filepair *p = q->queue[i];\n\t\t\tif (check_pair_status(p))\n\t\t\t\tdiff_flush_stat(p, options, &diffstat);\n\t\t}\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\tif (output_format & DIFF_FORMAT_SHORTSTAT)\n\t\t\tshow_shortstats(&diffstat, options);\n\t\tfree_diffstat_info(&diffstat);\n\t\tseparator++;\n\t}\n\nIn the loop, it calls diff_flush_stat() for each and every filepair in the\nqueue. The result is collected in &diffstat, and that is what your patch\nworks on in show_stats() function. Before you look at it in show_stats(),\nthe same &diffstat may be examined by show_numstat(), and after you look\nat it, the same &diffstat may be examined by show_shortstats().\n\ndiff_flush_stat() calls run_diffstat() to get textual patch for one\nfilepair. It ends up calling builtin_diffstat().\n\nbuiltin_diffstat() looks at the preimage and postimage, and feeds the low\nlevel xdiff machinery with it, and uses diffstat_consume() callbac to\ncount the +/- lines in the patch between the two.\n\nEven though three show_*stat* function look at the same &diffstat, two\nhave duplicated logic to ignore (and uncount) a path whose executable\nbit was changed without any other change, with:\n\n\tif (!it->is_renamed && (added + deleted) == 0)\n \tignore it!\n"},{"id":"168908","messageId":"7v39jzqvny.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"5da631c64438ec3f669f0c2b7456bcfbc371e2f5.1306499600.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/3] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-28T04:46:41Z","receivedAt":"2011-05-28T04:46:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> diff --git a/diff.c b/diff.c\n> index 4541939..6a6cbca 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1259,6 +1259,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \n> \twidth = options->stat_width ? options->stat_width : 80;\n> \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n> +\tcount = options->stat_count ? options->stat_count : data->nr;\n\nIt is a tad hard to follow the logic of \"count\" here. It could be the\nceiling the user gave you, or we may not be constrained at all if the full\ndiff is smaller. But then\n\n> @@ -1275,11 +1276,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \tadd_c = diff_get_color_opt(options, DIFF_FILE_NEW);\n> \tdel_c = diff_get_color_opt(options, DIFF_FILE_OLD);\n> \n> -\tfor (i = 0; i < data->nr; i++) {\n> +\tfor (i = 0; (i < count) && (i < data->nr); i++) {\n\nHere we compare \"i\" which does not have much to do with how many we\nactually have to show (as we may be skipping in the loop) with \"count\" and\nalso with data->nr. In the loop, you compensate for the skipping\n(i.e. increment of \"i\" that should not affect the outcome) by incrementing\n\"count\", so (i < count) part will correctly count towards the limit the\nuser gave. It may be a correct math, but it is tricky and hard to read.\nEspecially if we started with the options->stat_count that is larger than\nthe data->nr, of course \"i < count\" will always hold true with or without\nskipping, which also is correct but even trickier.\n\n> \t\tstruct diffstat_file *file = data->files[i];\n> \t\tuintmax_t change = file->added + file->deleted;\n> \t\tif (!data->files[i]->is_renamed &&\n> \t\t\t (change == 0)) {\n> +\t\t\tcount++; /* not shown == room for one more */\n> \t\t\tcontinue;\n> \t\t}\n> \t\tfill_print_name(file);\n> @@ -1292,6 +1294,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\tif (max_change < change)\n> \t\t\tmax_change = change;\n> \t}\n> +\tcount = i; /* min(count, data->nr) */\n\nThis value is mechanically min(count, data->nr) but it is confusing to\nexplain what it really means. It is the number of elements you have to\nlook at in data->files[] array, still skipping irrelevant entries in the\nloop, until you show options->stat_count number of entries.\n\n> @@ -1306,7 +1309,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \telse\n> \t\twidth = max_change;\n> \n> -\tfor (i = 0; i < data->nr; i++) {\n> +\tfor (i = 0; i < count; i++) {\n\nHence this loop is correct. You ignore everything beyond count in the\narray, as their lengths are immaterial to the outcome.\n\n> @@ -1373,6 +1376,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\tshow_graph(options->file, '-', del, del_c, reset);\n> \t\tfprintf(options->file, \"\\n\");\n> \t}\n> +\tif (count < data->nr)\n> +\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n\nAt this point, count is the sum of the number of items you are going to\nshow and the number of items you are going to skip. If that is smaller\nthan the total in the original array, there are items that you are not\nshowing nor skipping. But you terminated the first loop early before\nreaching the end of data->file[] array, so you do not know enough to say\n\"there _are_ more to show but I am not showing\", I think.\n\nIf\n\n - data->nr is 3;\n - the data->file[0] is to be shown but other entries in data->file[] are\n not to be shown; and\n - opt->stat_count is 1\n\nthen your first loop fills data->file[0]->name, increments i to 1, the\ncondition (i < count) no longer holds, and stops. At this point count is\n1, data->nr is 3, but data->file[1] and [2] are not to be shown anyway, so\nthis \"there are more but I am not showing them\" is not quite right, is it?\n\n> +\tfor (i = count; i < data->nr; i++) {\n> +\t\tuintmax_t added = data->files[i]->added;\n> +\t\tuintmax_t deleted = data->files[i]->deleted;\n> +\t\tif (!data->files[i]->is_renamed &&\n> +\t\t\t (added + deleted == 0)) {\n> +\t\t\ttotal_files--;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\t\tadds += added;\n> +\t\tdels += deleted;\n> +\t}\n\nIf this loop runs \"adds += added\" even once, then you have \"there are more\nbut I am not showing\" situation, and if all the entries beyond count are\nskipped, then you are not limiting the output.\n\nBy the way, in this round, you are trying to show only the first N items,\nand if there are more to show, then showing \"...\" after them. I kind of\nliked the idea that came up during the discussion in earlier rounds to\nshow N-1 then \"...\" then the last one item, but I do not feel too\nstrongly about it.\n\n diff.c | 6 ++++--\n t/t4049-diff-stat-count.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 82789e3..542ff42 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1247,6 +1247,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tint width, name_width, count;\n \tconst char *reset, *add_c, *del_c;\n \tconst char *line_prefix = \"\";\n+\tint extra_shown = 0;\n \tstruct strbuf *msg = NULL;\n \n \tif (data->nr == 0)\n@@ -1376,8 +1377,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tshow_graph(options->file, '-', del, del_c, reset);\n \t\tfprintf(options->file, \"\\n\");\n \t}\n-\tif (count < data->nr)\n-\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n \tfor (i = count; i < data->nr; i++) {\n \t\tuintmax_t added = data->files[i]->added;\n \t\tuintmax_t deleted = data->files[i]->deleted;\n@@ -1388,6 +1387,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t}\n \t\tadds += added;\n \t\tdels += deleted;\n+\t\tif (!extra_shown)\n+\t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n+\t\textra_shown = 1;\n \t}\n \tfprintf(options->file, \"%s\", line_prefix);\n \tfprintf(options->file,\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nnew file mode 100755\nindex 0000000..641e70d\n--- /dev/null\n+++ b/t/t4049-diff-stat-count.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+# Copyright (c) 2011, Google Inc.\n+\n+test_description='diff --stat-count'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t>a &&\n+\t>b &&\n+\t>c &&\n+\t>d &&\n+\tgit add a b c d &&\n+\tchmod +x c d &&\n+\techo a >a &&\n+\techo b >b &&\n+\tcat >expect <<-\\EOF\n+\t a | 1 +\n+\t b | 1 +\n+\t 2 files changed, 2 insertions(+), 0 deletions(-)\n+\tEOF\n+\tgit diff --stat --stat-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n"},{"id":"168976","messageId":"4DE33BF0.7060607@drmicha.warpmail.net","threadId":"27218","inReplyTo":"7v39jzqvny.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 2/3] diff: introduce --stat-lines to limit the stat lines","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-30T06:40:48Z","receivedAt":"2011-05-30T06:40:48Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 28.05.2011 06:46:\n> Michael J Gruber writes:\n> \n>> diff --git a/diff.c b/diff.c\n>> index 4541939..6a6cbca 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -1259,6 +1259,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \n>> \twidth = options->stat_width ? options->stat_width : 80;\n>> \tname_width = options->stat_name_width ? options->stat_name_width : 50;\n>> +\tcount = options->stat_count ? options->stat_count : data->nr;\n> \n> It is a tad hard to follow the logic of \"count\" here. It could be the\n> ceiling the user gave you, or we may not be constrained at all if the full\n> diff is smaller. But then\n> \n>> @@ -1275,11 +1276,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \tadd_c = diff_get_color_opt(options, DIFF_FILE_NEW);\n>> \tdel_c = diff_get_color_opt(options, DIFF_FILE_OLD);\n>> \n>> -\tfor (i = 0; i < data->nr; i++) {\n>> +\tfor (i = 0; (i < count) && (i < data->nr); i++) {\n> \n> Here we compare \"i\" which does not have much to do with how many we\n> actually have to show (as we may be skipping in the loop) with \"count\" and\n> also with data->nr. In the loop, you compensate for the skipping\n> (i.e. increment of \"i\" that should not affect the outcome) by incrementing\n> \"count\", so (i < count) part will correctly count towards the limit the\n> user gave. It may be a correct math, but it is tricky and hard to read.\n> Especially if we started with the options->stat_count that is larger than\n> the data->nr, of course \"i < count\" will always hold true with or without\n> skipping, which also is correct but even trickier.\n\nMaybe I was trying too hard to be efficient. We could have count be\nstat_count if non-zero or data->nr and limit all loops by the double\nconditional (i < count) && (i < data->nr), resp. use an additional\ncounter like shown or skipped and compare with that.\n\nGiven the short discussion for the initial patch, I really did not\nexpect how much it would still take... (My bad, of course.)\n> \n>> \t\tstruct diffstat_file *file = data->files[i];\n>> \t\tuintmax_t change = file->added + file->deleted;\n>> \t\tif (!data->files[i]->is_renamed &&\n>> \t\t\t (change == 0)) {\n>> +\t\t\tcount++; /* not shown == room for one more */\n>> \t\t\tcontinue;\n>> \t\t}\n>> \t\tfill_print_name(file);\n>> @@ -1292,6 +1294,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \t\tif (max_change < change)\n>> \t\t\tmax_change = change;\n>> \t}\n>> +\tcount = i; /* min(count, data->nr) */\n> \n> This value is mechanically min(count, data->nr) but it is confusing to\n> explain what it really means. It is the number of elements you have to\n> look at in data->files[] array, still skipping irrelevant entries in the\n> loop, until you show options->stat_count number of entries.\n\nAs you explain further down, we may have to look at even more in order\nto be able to say \"...\" or not.\n\n> \n>> @@ -1306,7 +1309,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \telse\n>> \t\twidth = max_change;\n>> \n>> -\tfor (i = 0; i < data->nr; i++) {\n>> +\tfor (i = 0; i < count; i++) {\n> \n> Hence this loop is correct. You ignore everything beyond count in the\n> array, as their lengths are immaterial to the outcome.\n> \n\nI think I really should not worry too much about efficiency here and\nmore about simple code.\n\n>> @@ -1373,6 +1376,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \t\tshow_graph(options->file, '-', del, del_c, reset);\n>> \t\tfprintf(options->file, \"\\n\");\n>> \t}\n>> +\tif (count < data->nr)\n>> +\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n> \n> At this point, count is the sum of the number of items you are going to\n> show and the number of items you are going to skip. If that is smaller\n> than the total in the original array, there are items that you are not\n> showing nor skipping. But you terminated the first loop early before\n> reaching the end of data->file[] array, so you do not know enough to say\n> \"there _are_ more to show but I am not showing\", I think.\n\nSigh, you're right. We would have to look until we find the first\nnon-show non-skipped in order to say that.\n\n> \n> If\n> \n> - data->nr is 3;\n> - the data->file[0] is to be shown but other entries in data->file[] are\n> not to be shown; and\n> - opt->stat_count is 1\n> \n> then your first loop fills data->file[0]->name, increments i to 1, the\n> condition (i < count) no longer holds, and stops. At this point count is\n> 1, data->nr is 3, but data->file[1] and [2] are not to be shown anyway, so\n> this \"there are more but I am not showing them\" is not quite right, is it?\n> \n>> +\tfor (i = count; i < data->nr; i++) {\n>> +\t\tuintmax_t added = data->files[i]->added;\n>> +\t\tuintmax_t deleted = data->files[i]->deleted;\n>> +\t\tif (!data->files[i]->is_renamed &&\n>> +\t\t\t (added + deleted == 0)) {\n>> +\t\t\ttotal_files--;\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\t\tadds += added;\n>> +\t\tdels += deleted;\n>> +\t}\n> \n> If this loop runs \"adds += added\" even once, then you have \"there are more\n> but I am not showing\" situation, and if all the entries beyond count are\n> skipped, then you are not limiting the output.\n> \n> By the way, in this round, you are trying to show only the first N items,\n> and if there are more to show, then showing \"...\" after them. I kind of\n> liked the idea that came up during the discussion in earlier rounds to\n> show N-1 then \"...\" then the last one item, but I do not feel too\n> strongly about it.\n\nI thought fmt-merge-msg was the example to follow. It does exactly what\nI'm trying here, i.e. show count items (if there are count or more), and\nshow \"...\" if there are more.\n\n> diff.c | 6 ++++--\n> t/t4049-diff-stat-count.sh | 25 +++++++++++++++++++++++++\n> 2 files changed, 29 insertions(+), 2 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 82789e3..542ff42 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1247,6 +1247,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \tint width, name_width, count;\n> \tconst char *reset, *add_c, *del_c;\n> \tconst char *line_prefix = \"\";\n> +\tint extra_shown = 0;\n> \tstruct strbuf *msg = NULL;\n> \n> \tif (data->nr == 0)\n> @@ -1376,8 +1377,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\tshow_graph(options->file, '-', del, del_c, reset);\n> \t\tfprintf(options->file, \"\\n\");\n> \t}\n> -\tif (count < data->nr)\n> -\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n> \tfor (i = count; i < data->nr; i++) {\n> \t\tuintmax_t added = data->files[i]->added;\n> \t\tuintmax_t deleted = data->files[i]->deleted;\n> @@ -1388,6 +1387,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t}\n> \t\tadds += added;\n> \t\tdels += deleted;\n> +\t\tif (!extra_shown)\n> +\t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n> +\t\textra_shown = 1;\n> \t}\n> \tfprintf(options->file, \"%s\", line_prefix);\n> \tfprintf(options->file,\n> diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\n> new file mode 100755\n> index 0000000..641e70d\n> --- /dev/null\n> +++ b/t/t4049-diff-stat-count.sh\n> @@ -0,0 +1,25 @@\n> +#!/bin/sh\n> +# Copyright (c) 2011, Google Inc.\n> +\n> +test_description='diff --stat-count'\n> +. ./test-lib.sh\n> +\n> +test_expect_success setup '\n> +\t>a &&\n> +\t>b &&\n> +\t>c &&\n> +\t>d &&\n> +\tgit add a b c d &&\n> +\tchmod +x c d &&\n> +\techo a >a &&\n> +\techo b >b &&\n> +\tcat >expect <<-\\EOF\n> +\t a | 1 +\n> +\t b | 1 +\n> +\t 2 files changed, 2 insertions(+), 0 deletions(-)\n> +\tEOF\n> +\tgit diff --stat --stat-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n\nThat works, thanks also for the test.\n\nShould I squash this in or go for a clearer use of \"count\"? (Also, I may\nhave to take into account your notes about the workflow.)\n\nMichael\n"},{"id":"168979","messageId":"7vhb8cod19.fsf@alter.siamese.dyndns.org","threadId":"27218","inReplyTo":"4DE33BF0.7060607@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/3] diff: introduce --stat-lines to limit the stat lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-30T07:36:34Z","receivedAt":"2011-05-30T07:36:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> That works, thanks also for the test.\n>\n> Should I squash this in or go for a clearer use of \"count\"? (Also, I may\n> have to take into account your notes about the workflow.)\n\nI do not recall any \"notes about the workflow\" but if you feel it is worth\nkeeping, please go ahead. I've queued the fix-up as a \"finishing touches\"\nseparate commit, but the series is still in 'pu' so it is a fair game for\nyou to do whatever you think is the most appropriate for it.\n\nThanks.\n"}]}