{"thread":{"id":"39771","subject":"[PATCH v2 0/4] git log --count","startedAt":"2015-07-02T23:50:07Z","lastAt":"2015-07-04T00:57:11Z","messageCount":17,"participants":["Lawrence Siebert","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"265441","messageId":"1435881011-13879-1-git-send-email-lawrencesiebert@gmail.com","threadId":"39771","inReplyTo":null,"subject":"[PATCH v2 0/4] git log --count","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-02T23:50:07Z","receivedAt":"2015-07-02T23:50:07Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"This should fix the whitespace issues this had previously. \n\nLawrence Siebert (4):\n list-object: add get_commit_count function\n log: add --count option to git log\n log --count: added test\n git-log: update man documentation for --count\n\n Documentation/git-log.txt | 2 ++\n Documentation/rev-list-options.txt | 2 +-\n builtin/log.c | 29 +++++++++++++++++++++++++++++\n builtin/rev-list.c | 12 ++----------\n list-objects.c | 14 ++++++++++++++\n list-objects.h | 1 +\n t/t4202-log.sh | 7 +++++++\n 7 files changed, 56 insertions(+), 11 deletions(-)\n\n-- \n1.9.1\n"},{"id":"265442","messageId":"1435881011-13879-2-git-send-email-lawrencesiebert@gmail.com","threadId":"39771","inReplyTo":"1435881011-13879-1-git-send-email-lawrencesiebert@gmail.com","subject":"[PATCH v2 1/4] list-object: add get_commit_count function","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-02T23:50:08Z","receivedAt":"2015-07-02T23:50:08Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"Moving commit counting from rev-list into list-object which is a step\ntoward letting git log do counting as well.\n\nSigned-off-by: Lawrence Siebert \n---\n builtin/rev-list.c | 12 ++----------\n list-objects.c | 14 ++++++++++++++\n list-objects.h | 1 +\n 3 files changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ff84a82..07f522b 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -388,16 +388,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \ttraverse_commit_list(&revs, show_commit, show_object, &info);\n \n-\tif (revs.count) {\n-\t\tif (revs.left_right && revs.cherry_mark)\n-\t\t\tprintf(\"%d\\t%d\\t%d\\n\", revs.count_left, revs.count_right, revs.count_same);\n-\t\telse if (revs.left_right)\n-\t\t\tprintf(\"%d\\t%d\\n\", revs.count_left, revs.count_right);\n-\t\telse if (revs.cherry_mark)\n-\t\t\tprintf(\"%d\\t%d\\n\", revs.count_left + revs.count_right, revs.count_same);\n-\t\telse\n-\t\t\tprintf(\"%d\\n\", revs.count_left + revs.count_right);\n-\t}\n+\tif (revs.count)\n+\t\tget_commit_count(&revs);\n \n \treturn 0;\n }\ndiff --git a/list-objects.c b/list-objects.c\nindex 41736d2..6f76301 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -234,3 +234,17 @@ void traverse_commit_list(struct rev_info *revs,\n \tobject_array_clear(&revs->pending);\n \tstrbuf_release(&base);\n }\n+\n+void get_commit_count(struct rev_info * revs) {\n+\tif (revs->count) {\n+\t\tif (revs->left_right && revs->cherry_mark)\n+\t\t\tprintf(\"%d\\t%d\\t%d\\n\", revs->count_left, revs->count_right, revs->count_same);\n+\t\telse if (revs->left_right)\n+\t\t\tprintf(\"%d\\t%d\\n\", revs->count_left, revs->count_right);\n+\t\telse if (revs->cherry_mark)\n+\t\t\tprintf(\"%d\\t%d\\n\", revs->count_left + revs->count_right, revs->count_same);\n+\t\telse\n+\t\t\tprintf(\"%d\\n\", revs->count_left + revs->count_right);\n+\t}\n+\treturn;\n+}\ndiff --git a/list-objects.h b/list-objects.h\nindex 136a1da..d28c1f3 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -7,5 +7,6 @@ void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, voi\n \n typedef void (*show_edge_fn)(struct commit *);\n void mark_edges_uninteresting(struct rev_info *, show_edge_fn);\n+void get_commit_count(struct rev_info * revs);\n \n #endif\n-- \n1.9.1\n"},{"id":"265445","messageId":"1435881011-13879-3-git-send-email-lawrencesiebert@gmail.com","threadId":"39771","inReplyTo":"1435881011-13879-1-git-send-email-lawrencesiebert@gmail.com","subject":"[PATCH v2 2/4] log: add --count option to git log","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-02T23:50:09Z","receivedAt":"2015-07-02T23:50:09Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"adds --count from git rev-list to git log, without --use-bitmap-index\nfor the moment.\n\nSigned-off-by: Lawrence Siebert \n---\n builtin/log.c | 29 +++++++++++++++++++++++++++++\n 1 file changed, 29 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8781049..4aaff3a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -25,6 +25,7 @@\n #include \"version.h\"\n #include \"mailmap.h\"\n #include \"gpg-interface.h\"\n+#include \"list-objects.h\"\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -317,12 +318,40 @@ static void finish_early_output(struct rev_info *rev)\n \tshow_early_header(rev, \"done\", n);\n }\n \n+static void show_object(struct object *obj,\n+\t\t\tconst struct name_path *path, const char *component,\n+\t\t\tvoid *cb_data)\n+{\n+\treturn;\n+}\n+\n+static void show_commit(struct commit *commit, void *data)\n+{\n+\tstruct rev_info *revs = (struct rev_info *)data;\n+\tif (commit->object.flags & PATCHSAME)\n+\t\trevs->count_same++;\n+\telse if (commit->object.flags & SYMMETRIC_LEFT)\n+\t\trevs->count_left++;\n+\telse\n+\t\trevs->count_right++;\n+\tif (commit->parents) {\n+\t\tfree_commit_list(commit->parents);\n+\t\tcommit->parents = NULL;\n+\t}\n+\tfree_commit_buffer(commit);\n+}\n+\n static int cmd_log_walk(struct rev_info *rev)\n {\n \tstruct commit *commit;\n \tint saved_nrl = 0;\n \tint saved_dcctc = 0;\n \n+\tif (rev->count) {\n+\t\tprepare_revision_walk(rev);\n+\t\ttraverse_commit_list(rev, show_commit, show_object, rev);\n+\t\tget_commit_count(rev);\n+\t}\n \tif (rev->early_output)\n \t\tsetup_early_output(rev);\n \n-- \n1.9.1\n"},{"id":"265443","messageId":"1435881011-13879-4-git-send-email-lawrencesiebert@gmail.com","threadId":"39771","inReplyTo":"1435881011-13879-1-git-send-email-lawrencesiebert@gmail.com","subject":"[PATCH v2 3/4] log --count: added test","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-02T23:50:10Z","receivedAt":"2015-07-02T23:50:10Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"added test comparing output between git log --count HEAD and\ngit rev-list --count HEAD\n\nSigned-off-by: Lawrence Siebert \n---\n t/t4202-log.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 1b2e981..35f8d82 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n \ttest_must_fail git log --graph --no-walk\n '\n \n+test_expect_success 'log --count' '\n+\tgit log --count HEAD > actual &&\n+\tgit\trev-list --count HEAD > expect &&\n+\ttest_cmp expect actual\n+'\n+\n+\n test_done\n-- \n1.9.1\n"},{"id":"265444","messageId":"1435881011-13879-5-git-send-email-lawrencesiebert@gmail.com","threadId":"39771","inReplyTo":"1435881011-13879-1-git-send-email-lawrencesiebert@gmail.com","subject":"[PATCH v2 4/4] git-log: update man documentation for --count","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-02T23:50:11Z","receivedAt":"2015-07-02T23:50:11Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"I'm not altogether sure the best way to update the internal usage\nfrom git-log -h, but this at least updates the man page.\n\nSigned-off-by: Lawrence Siebert \n---\n Documentation/git-log.txt | 2 ++\n Documentation/rev-list-options.txt | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 5692945..0e10e44 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -90,6 +90,8 @@ include::line-range-format.txt[]\n Paths may need to be prefixed with ``\\-- '' to separate them from\n options or the revision range, when confusion arises.\n \n+\n+:git-log: 1\n include::rev-list-options.txt[]\n \n include::pretty-formats.txt[]\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 77ac439..f4e15fb 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -797,7 +797,7 @@ This implies the `--topo-order` option by default, but the\n \tin between them in that case. If `` is specified, it\n \tis the string that will be shown instead of the default one.\n \n-ifdef::git-rev-list[]\n+ifdef::git-log,git-rev-list[]\n --count::\n \tPrint a number stating how many commits would have been\n \tlisted, and suppress all other output. When used together\n-- \n1.9.1\n"},{"id":"265448","messageId":"vpqlhex66v6.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"1435881011-13879-2-git-send-email-lawrencesiebert@gmail.com","subject":"Re: [PATCH v2 1/4] list-object: add get_commit_count function","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T07:24:45Z","receivedAt":"2015-07-03T07:24:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Lawrence Siebert writes:\n\n> +void get_commit_count(struct rev_info * revs) {\n\nPlease, write \"struct rev_info *revs\" (stick * to revs).\n\n> +void get_commit_count(struct rev_info * revs);\n\nLikewise.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265449","messageId":"vpqa8vd66ms.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"1435881011-13879-3-git-send-email-lawrencesiebert@gmail.com","subject":"Re: [PATCH v2 2/4] log: add --count option to git log","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T07:29:47Z","receivedAt":"2015-07-03T07:29:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Lawrence Siebert writes:\n\n> +static void show_object(struct object *obj,\n> +\t\t\tconst struct name_path *path, const char *component,\n> +\t\t\tvoid *cb_data)\n> +{\n> +\treturn;\n> +}\n\nIt seems streange to me to have a function named show_object when it\ndoes not show anything. Maybe name it null_travers_cb to make it clear\nit's a callback and it does nothing?\n\nNot a strong objection though, it's only a static function.\n\n> +static void show_commit(struct commit *commit, void *data)\n> +{\n> +\tstruct rev_info *revs = (struct rev_info *)data;\n> +\tif (commit->object.flags & PATCHSAME)\n> +\t\trevs->count_same++;\n> +\telse if (commit->object.flags & SYMMETRIC_LEFT)\n> +\t\trevs->count_left++;\n> +\telse\n> +\t\trevs->count_right++;\n> +\tif (commit->parents) {\n> +\t\tfree_commit_list(commit->parents);\n> +\t\tcommit->parents = NULL;\n> +\t}\n> +\tfree_commit_buffer(commit);\n> +}\n> +\n> static int cmd_log_walk(struct rev_info *rev)\n> {\n> \tstruct commit *commit;\n> \tint saved_nrl = 0;\n> \tint saved_dcctc = 0;\n> \n> +\tif (rev->count) {\n> +\t\tprepare_revision_walk(rev);\n> +\t\ttraverse_commit_list(rev, show_commit, show_object, rev);\n> +\t\tget_commit_count(rev);\n> +\t}\n\nI didn't test, but it seems this does a full graph traversal before\nstarting the output. A very important property of \"git log\" is that it\nstarts showing revisions immediately, even when ran on a very long\nhistory (it shows the first screen immediately and continues working in\nthe background while the first page is displayed in the pager).\n\nIs it the case? If so, it should be changed. If not, perhaps explain why\nin the commit message.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265450","messageId":"vpqwpyh4rup.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"1435881011-13879-4-git-send-email-lawrencesiebert@gmail.com","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T07:34:22Z","receivedAt":"2015-07-03T07:34:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Lawrence Siebert writes:\n\n> added test comparing output between git log --count HEAD and\n> git rev-list --count HEAD\n\nUnless there is a very long list of tests, I'd rather see this squashed\nwith PATCH 2/4. As a reviewer I prefer having code and tests in the same\nplace.\n\n> Signed-off-by: Lawrence Siebert \n> ---\n> t/t4202-log.sh | 7 +++++++\n> 1 file changed, 7 insertions(+)\n>\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 1b2e981..35f8d82 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n> \ttest_must_fail git log --graph --no-walk\n> '\n> \n> +test_expect_success 'log --count' '\n> +\tgit log --count HEAD > actual &&\n> +\tgit\trev-list --count HEAD > expect &&\n\nThe weird space is still there.\n\nAlso, we write \">actual\", not \"> actual\" in the Git coding style.\n\nThat is actually a rather weak test. rev-list --count interacts with\n--left-right, so I guess you want to test --count --left-right.\n\nAlso, some revision-limiting options can reduce the count like\n\ngit log --grep whatever\n\nand you should check that you actually count the right number here.\n\n(I don't know this part of the code enough, but I'm not sure you\nactually deal with this properly)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265451","messageId":"CAKDoJU5vHP8c+Cvk1vBjjMG3kHD_rN3XwmfVngMYLN2Tkcg-tQ@mail.gmail.com","threadId":"39771","inReplyTo":"vpqa8vd66ms.fsf@anie.imag.fr","subject":"Re: [PATCH v2 2/4] log: add --count option to git log","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-03T08:05:50Z","receivedAt":"2015-07-03T08:05:50Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"traverse_commit_list requires a function to be passed to it. That\nsaid, after further review it can probably pass NULL and have it work\nfine. If not, I'll use your naming convention.\n\n`git rev-list --count` (or actually `git rev-list --count HEAD`, which\nthis borrows the code from, produces a single value, a numeric count.\nI think walking the entire list is necessary to get the final value,\nwhich is what we want with --count.\n\nThanks,\nLawrence Siebert\n\n\n\n\n\nOn Fri, Jul 3, 2015 at 12:29 AM, Matthieu Moy\n wrote:\n> Lawrence Siebert writes:\n>\n>> +static void show_object(struct object *obj,\n>> + const struct name_path *path, const char *component,\n>> + void *cb_data)\n>> +{\n>> + return;\n>> +}\n>\n> It seems streange to me to have a function named show_object when it\n> does not show anything. Maybe name it null_travers_cb to make it clear\n> it's a callback and it does nothing?\n>\n> Not a strong objection though, it's only a static function.\n>\n>> +static void show_commit(struct commit *commit, void *data)\n>> +{\n>> + struct rev_info *revs = (struct rev_info *)data;\n>> + if (commit->object.flags & PATCHSAME)\n>> + revs->count_same++;\n>> + else if (commit->object.flags & SYMMETRIC_LEFT)\n>> + revs->count_left++;\n>> + else\n>> + revs->count_right++;\n>> + if (commit->parents) {\n>> + free_commit_list(commit->parents);\n>> + commit->parents = NULL;\n>> + }\n>> + free_commit_buffer(commit);\n>> +}\n>> +\n>> static int cmd_log_walk(struct rev_info *rev)\n>> {\n>> struct commit *commit;\n>> int saved_nrl = 0;\n>> int saved_dcctc = 0;\n>>\n>> + if (rev->count) {\n>> + prepare_revision_walk(rev);\n>> + traverse_commit_list(rev, show_commit, show_object, rev);\n>> + get_commit_count(rev);\n>> + }\n>\n> I didn't test, but it seems this does a full graph traversal before\n> starting the output. A very important property of \"git log\" is that it\n> starts showing revisions immediately, even when ran on a very long\n> history (it shows the first screen immediately and continues working in\n> the background while the first page is displayed in the pager).\n>\n> Is it the case? If so, it should be changed. If not, perhaps explain why\n> in the commit message.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n-- \nAbout Me: http://about.me/lawrencesiebert\nConstantly Coding: http://constantcoding.blogspot.com\n"},{"id":"265452","messageId":"CAKDoJU6DdSc2ux5vEJu2risW9a-cgJ7e5xBxysTFdYGMd+77Kw@mail.gmail.com","threadId":"39771","inReplyTo":"vpqlhex66v6.fsf@anie.imag.fr","subject":"Re: [PATCH v2 1/4] list-object: add get_commit_count function","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-03T08:09:39Z","receivedAt":"2015-07-03T08:09:39Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"Mattieu,\n\nUnderstood. I don't suppose there is any commonly code formatting tool\nto automate formatting in the git style, is there?\n\nThanks,\nLawrence\n\nOn Fri, Jul 3, 2015 at 12:24 AM, Matthieu Moy\n wrote:\n> Lawrence Siebert writes:\n>\n>> +void get_commit_count(struct rev_info * revs) {\n>\n> Please, write \"struct rev_info *revs\" (stick * to revs).\n>\n>> +void get_commit_count(struct rev_info * revs);\n>\n> Likewise.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n-- \nAbout Me: http://about.me/lawrencesiebert\nConstantly Coding: http://constantcoding.blogspot.com\n"},{"id":"265453","messageId":"CAKDoJU7MT5PkoPKaF=NYne0sgNfGC6q+y7+57L0135dgxKThgw@mail.gmail.com","threadId":"39771","inReplyTo":"vpqwpyh4rup.fsf@anie.imag.fr","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-03T08:30:44Z","receivedAt":"2015-07-03T08:30:44Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"Matthieu,\n\nOk, I'll fix that. I think I can also add tests, I can look at the\ntests for rev-list --count, with the understanding that I saw somebody\nelse had made changes for the --use-bitmap-index option, and I am\nbasing off of master for this, and thus don't feel comfortable with\n--use-bitmap-index at this time.\n\nIf it's acceptable practice, I'll just squash everything I do on this\nfeature and it's tests into one commit with a more detailed comment,\nand send the patch for that. I wasn't sure about how much history I\nshould save, and how much I should split stuff up, so I appreciate\nyour clarification.\n\nThank you for your time,\nLawrence Siebert\n\nOn Fri, Jul 3, 2015 at 12:34 AM, Matthieu Moy\n wrote:\n> Lawrence Siebert writes:\n>\n>> added test comparing output between git log --count HEAD and\n>> git rev-list --count HEAD\n>\n> Unless there is a very long list of tests, I'd rather see this squashed\n> with PATCH 2/4. As a reviewer I prefer having code and tests in the same\n> place.\n>\n>> Signed-off-by: Lawrence Siebert \n>> ---\n>> t/t4202-log.sh | 7 +++++++\n>> 1 file changed, 7 insertions(+)\n>>\n>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n>> index 1b2e981..35f8d82 100755\n>> --- a/t/t4202-log.sh\n>> +++ b/t/t4202-log.sh\n>> @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n>> test_must_fail git log --graph --no-walk\n>> '\n>>\n>> +test_expect_success 'log --count' '\n>> + git log --count HEAD > actual &&\n>> + git rev-list --count HEAD > expect &&\n>\n> The weird space is still there.\n>\n> Also, we write \">actual\", not \"> actual\" in the Git coding style.\n>\n> That is actually a rather weak test. rev-list --count interacts with\n> --left-right, so I guess you want to test --count --left-right.\n>\n> Also, some revision-limiting options can reduce the count like\n>\n> git log --grep whatever\n>\n> and you should check that you actually count the right number here.\n>\n> (I don't know this part of the code enough, but I'm not sure you\n> actually deal with this properly)\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n-- \nAbout Me: http://about.me/lawrencesiebert\nConstantly Coding: http://constantcoding.blogspot.com\n"},{"id":"265454","messageId":"vpq7fqh4nxb.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"CAKDoJU6DdSc2ux5vEJu2risW9a-cgJ7e5xBxysTFdYGMd+77Kw@mail.gmail.com","subject":"Re: [PATCH v2 1/4] list-object: add get_commit_count function","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T08:59:12Z","receivedAt":"2015-07-03T08:59:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Lawrence Siebert writes:\n\n> Mattieu,\n>\n> Understood. I don't suppose there is any commonly code formatting tool\n> to automate formatting in the git style, is there?\n\nIIRC, someone posted a configuration file for clang-format that\nessentially matches the Git coding style.\n\nYou can read Documentation/CodingGuidelines. Unsurprisingly, Git uses a\ncoding style similar to the Linux kernel, so anything you find about the\nkernel essentially applies here too.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265455","messageId":"vpqfv5538jw.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"CAKDoJU5vHP8c+Cvk1vBjjMG3kHD_rN3XwmfVngMYLN2Tkcg-tQ@mail.gmail.com","subject":"Re: [PATCH v2 2/4] log: add --count option to git log","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T09:16:35Z","receivedAt":"2015-07-03T09:16:35Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Hi,\n\nPlease, don't top-post on this list. Quote the parts you're replying to\nand reply below.\n\nLawrence Siebert writes:\n\n> traverse_commit_list requires a function to be passed to it. That\n> said, after further review it can probably pass NULL and have it work\n> fine. If not, I'll use your naming convention.\n\nIf possible, passing NULL would be best.\n\n> `git rev-list --count` (or actually `git rev-list --count HEAD`, which\n> this borrows the code from, produces a single value, a numeric count.\n> I think walking the entire list is necessary to get the final value,\n> which is what we want with --count.\n\nOK, that was me answering emails before coffee. I thought --count was\nproducing the count _in addition_ to the normal output. But then I don't\nunderstand by looking at the code how you prevent the normal output. I\njust tested, and indeed it does work (I guess all the magic is already\nthere from \"rev-list --count\", and it was more or less a bug that \"log\n--count\" was not using it properly), but you may want to explain better\nwhat's going on in the commit message.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265456","messageId":"vpqegkp38jg.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"CAKDoJU7MT5PkoPKaF=NYne0sgNfGC6q+y7+57L0135dgxKThgw@mail.gmail.com","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T09:16:51Z","receivedAt":"2015-07-03T09:16:51Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Lawrence Siebert writes:\n\n> Ok, I'll fix that.\n\n(Note: this is a typical example of why we don't top-post here. I made\nseveral remarks and I can't know what \"that\" refers to)\n\n(Meta-note: don't take the note as agressive, I know that top-posting is\nthe norm in many other places, I'm just giving you a glimpse of the\nlocal culture ;-) ).\n\n> If it's acceptable practice, I'll just squash everything I do on this\n> feature and it's tests into one commit with a more detailed comment,\n> and send the patch for that.\n\nI think at least two patches are better: your PATCH 1 is a typical\npreparation step, best reviewed alone in its own patch.\n\nSplitting history into several patches is good, but each patch should\ncorrespond to one logical step. Splitting code Vs doc Vs tests is\nusually not the right way.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265457","messageId":"vpqpp491s7v.fsf@anie.imag.fr","threadId":"39771","inReplyTo":"vpqwpyh4rup.fsf@anie.imag.fr","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-03T09:54:44Z","receivedAt":"2015-07-03T09:54:44Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Matthieu Moy writes:\n\n> Also, some revision-limiting options can reduce the count like\n>\n> git log --grep whatever\n\nOK, --grep seems to work, but -S and -G do not:\n\n$ ./bin-wrappers/git log -Sfoo --count\n40012\n$ ./bin-wrappers/git log -Sfoo --oneline | wc -l\n925\n$ ./bin-wrappers/git log --count \n40012\n\nSee 251df09 (log: fix --max-count when used together with -S or -G,\n2011-03-09) for a start of an explanation.\n\nIf implementing a proper count is too hard, one option is to forbid\n--count -S and --count -G to avoid confusion.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265505","messageId":"xmqqd2084u8y.fsf@gitster.dls.corp.google.com","threadId":"39771","inReplyTo":"vpqwpyh4rup.fsf@anie.imag.fr","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-04T00:54:53Z","receivedAt":"2015-07-04T00:54:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Also, some revision-limiting options can reduce the count like\n>\n> git log --grep whatever\n>\n> and you should check that you actually count the right number here.\n>\n> (I don't know this part of the code enough, but I'm not sure you\n> actually deal with this properly)\n\nYes, \"rev-list\", when the revision range is \"limited\" (with or\nwithout pathspec), can give \"--count\" once limit_list() finishes,\nbut \"log\" filters the result of limit_list() further with at least\nthree separate phases.\n\n - options in the \"grep\" family (--grep/--author/etc.) lets you skip\n commits based on the contents of the commit object;\n\n - options in the \"diff\" family (-w/-b/etc.) may let \"git log\"\n consider a commit because the pathspec limit thought two blobs\n were different at byte-by-byte level, but after running \"diff\"\n with these \"looser\" comparison, \"git log\" may realize that there\n weren't any interesting change introduced by the commit [*1*];\n\n - and finally, of course \"log --max-count=20\" may further limit the\n maximum number of commits that are shown. This of course is not\n interesting in the context of \"--count\" in the sense that \"git\n log --count -20 --grep=foo maint..master\" may not be immediately\n a sensible thing to do (but we never know. Perhaps your user may\n be asking \"do we have 20 or more commits that say 'foo' in the\n range?\")\n\nAn implementation of \"--count\" to take the first and the third ones\nin account may not be too hard, but I am fairly familiar with the\ncodepath for the second one and I think it would be very tricky.\n\nNote that these additional things \"log\" does over \"rev-list\" *DO*\njustify addition of \"--count\" to \"log\" (because \"rev-list --count\"\ncannot emulate these); I am however not sure if it is worth the\nadditional complexity we need to add to the codepath (especially for\nthe second phase). I'd need to take another look at the codepaths\ninvolved myself to be sure, but I suspect the damage to the codepath\nfor the second may end up to be extensive when we do decide to fix\nthe possible bug in it.\n\n\n[Footnote]\n\n*1* They may still show the log message in such a case where \"-b/-w\"\nwas asked and commit had only whitespace changes, but I think we\nshould consider that a bug.\n"},{"id":"265506","messageId":"xmqq8uaw4u54.fsf@gitster.dls.corp.google.com","threadId":"39771","inReplyTo":"vpqpp491s7v.fsf@anie.imag.fr","subject":"Re: [PATCH v2 3/4] log --count: added test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-04T00:57:11Z","receivedAt":"2015-07-04T00:57:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> If implementing a proper count is too hard, one option is to forbid\n> --count -S and --count -G to avoid confusion.\n\nLet's not go there. Letting people to use \"--oneline | wc -l\" is\nfar better unless we can get --count that behaves the same as that,\nonly faster.\n"}]}