{"thread":{"id":"41201","subject":"[PATCH 0/6] shortlog fixes and optimizations","startedAt":"2016-01-15T17:06:28Z","lastAt":"2016-01-19T03:47:54Z","messageCount":22,"participants":["Jeff King","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"276167","messageId":"20160115170627.GA20983@sigill.intra.peff.net","threadId":"41201","inReplyTo":null,"subject":"[PATCH 0/6] shortlog fixes and optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:06:28Z","receivedAt":"2016-01-15T17:06:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are split out from my the shortlog-trailer series I sent a few\nweeks ago. The \"trailer\" parts still need some re-working, but there is\nno need to hold these fixes hostage.\n\nI also dropped the early part of the series, adding skip_prefix_icase().\nAfter digging into the history, I ended up reworking the first patch\nhere to do a more thorough parse, so we don't need it anymore.\n\nThanks to Eric Sunshine for review on the first iteration; this\nincorporates his comments.\n\n [1/6]: shortlog: match both \"Author:\" and \"author\" on stdin\n [2/6]: shortlog: use strbufs to read from stdin\n [3/6]: shortlog: replace hand-parsing of author with pretty-printer\n [4/6]: shortlog: optimize \"--summary\" mode\n [5/6]: shortlog: optimize out useless \"\" normalization\n [6/6]: shortlog: optimize out useless string list\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/283091\n"},{"id":"276168","messageId":"20160115170823.GA21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:08:23Z","receivedAt":"2016-01-15T17:08:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The original git-shortlog could read both the normal \"git\nlog\" output as well as \"git log --format=raw\". However, when it was\nconverted to C by b8ec592 (Build in shortlog, 2006-10-22),\nthe trailing colon became mandatory, and we no longer\nmatched the raw output.\n\nGiven the amount of intervening time without any bug\nreports, it's probable that nobody cares. But given that\nit's easy to fix, and that the end result is hopefully more\nobvious and flexible (it could now easily accomodate matching\n\"Committer\"), let's just make it work.\n\nSigned-off-by: Jeff King \n---\nAnother option would be to assume that nobody cares about\n\"--format=raw\" and just do:\n\n if (!skip_prefix(author, \"Author: \", &v))\n\tcontinue;\n\nThat technically breaks somebody who was feeding shortlog output that\ncontains \"author: \", but since Git itself doesn't generate that, it\nseems rather unlikely.\n\n builtin/shortlog.c | 27 ++++++++++++++++++++++++---\n t/t4201-shortlog.sh | 6 ++++++\n 2 files changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 35ebd17..fe9fa2f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -89,13 +89,34 @@ static void insert_one_record(struct shortlog *log,\n \tstring_list_append(item->util, buffer);\n }\n \n+/*\n+ * If header is \"author\", match candidate against the regex /[Aa]uthor:? /,\n+ * and return a pointer to the remainder of the string in out_value.\n+ */\n+static int match_ident_header(const char *candidate, const char *header,\n+\t\t\t const char **out_value)\n+{\n+\tconst char *v;\n+\n+\tif (tolower(*candidate++) != tolower(*header++))\n+\t\treturn 0;\n+\tif (!skip_prefix(candidate, header, &v))\n+\t\treturn 0;\n+\tif (*v == ':')\n+\t\tv++;\n+\tif (*v++ != ' ')\n+\t\treturn 0;\n+\t*out_value = v;\n+\treturn 1;\n+}\n+\n static void read_from_stdin(struct shortlog *log)\n {\n \tchar author[1024], oneline[1024];\n \n \twhile (fgets(author, sizeof(author), stdin) != NULL) {\n-\t\tif (!(author[0] == 'A' || author[0] == 'a') ||\n-\t\t !starts_with(author + 1, \"uthor: \"))\n+\t\tconst char *v;\n+\t\tif (!match_ident_header(author, \"author\", &v))\n \t\t\tcontinue;\n \t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n \t\t oneline[0] != '\\n')\n@@ -103,7 +124,7 @@ static void read_from_stdin(struct shortlog *log)\n \t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n \t\t oneline[0] == '\\n')\n \t\t\t; /* discard blanks */\n-\t\tinsert_one_record(log, author + 8, oneline);\n+\t\tinsert_one_record(log, v, oneline);\n \t}\n }\n \ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 7600a3e..82b2314 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -120,6 +120,12 @@ test_expect_success !MINGW 'shortlog from non-git directory' '\n \ttest_cmp expect out\n '\n \n+test_expect_success !MINGW 'shortlog can read --format=raw output' '\n+\tgit log --format=raw HEAD >log &&\n+\tGIT_DIR=non-existing git shortlog -w out &&\n+\ttest_cmp expect out\n+'\n+\n test_expect_success 'shortlog should add newline when input line matches wraplen' '\n \tcat >expect <<\\EOF &&\n A U Thor (2):\n-- \n2.7.0.244.g0701a9d\n"},{"id":"276169","messageId":"20160115170835.GB21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 2/6] shortlog: use strbufs to read from stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:08:35Z","receivedAt":"2016-01-15T17:08:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We currently use fixed-size buffers with fgets(), which\ncould lead to incorrect results in the unlikely event that a\nline had something like \"Author:\" at exactly its 1024th\ncharacter.\n\nBut it's easy to convert this to a strbuf, and because we\ncan reuse the same buffer through the loop, we don't even\npay the extra allocation cost.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex fe9fa2f..bb2ee5b 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -112,20 +112,23 @@ static int match_ident_header(const char *candidate, const char *header,\n \n static void read_from_stdin(struct shortlog *log)\n {\n-\tchar author[1024], oneline[1024];\n+\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf oneline = STRBUF_INIT;\n \n-\twhile (fgets(author, sizeof(author), stdin) != NULL) {\n+\twhile (strbuf_getline(&author, stdin, '\\n') != EOF) {\n \t\tconst char *v;\n-\t\tif (!match_ident_header(author, \"author\", &v))\n+\t\tif (!match_ident_header(author.buf, \"author\", &v))\n \t\t\tcontinue;\n-\t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n-\t\t oneline[0] != '\\n')\n+\t\twhile (strbuf_getline(&oneline, stdin, '\\n') != EOF &&\n+\t\t oneline.len)\n \t\t\t; /* discard headers */\n-\t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n-\t\t oneline[0] == '\\n')\n+\t\twhile (strbuf_getline(&oneline, stdin, '\\n') != EOF &&\n+\t\t !oneline.len)\n \t\t\t; /* discard blanks */\n-\t\tinsert_one_record(log, v, oneline);\n+\t\tinsert_one_record(log, v, oneline.buf);\n \t}\n+\tstrbuf_release(&author);\n+\tstrbuf_release(&oneline);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n-- \n2.7.0.244.g0701a9d\n"},{"id":"276170","messageId":"20160115170903.GC21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:09:03Z","receivedAt":"2016-01-15T17:09:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When gathering the author and oneline subject for each\ncommit, we hand-parse the commit headers to find the\n\"author\" line, and then continue past to the blank line at\nthe end of the header.\n\nWe can replace this tricky hand-parsing by simply asking the\npretty-printer for the relevant items. This also decouples\nthe author and oneline parsing, opening up some new\noptimizations in further commits.\n\nOne reason to avoid the pretty-printer is that it might be\nless efficient than hand-parsing. However, I measured no\nslowdown at all running \"git shortlog -ns HEAD\" on\nlinux.git.\n\nAs a bonus, we also fix a memory leak in the (uncommon) case\nthat the author field is blank.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 62 +++++++++++++++++++++++-------------------------------\n 1 file changed, 26 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex bb2ee5b..256b868 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -133,45 +133,35 @@ static void read_from_stdin(struct shortlog *log)\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n-\tconst char *author = NULL, *buffer;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tstruct strbuf ufbuf = STRBUF_INIT;\n-\n-\tpp_commit_easy(CMIT_FMT_RAW, commit, &buf);\n-\tbuffer = buf.buf;\n-\twhile (*buffer && *buffer != '\\n') {\n-\t\tconst char *eol = strchr(buffer, '\\n');\n-\n-\t\tif (eol == NULL)\n-\t\t\teol = buffer + strlen(buffer);\n-\t\telse\n-\t\t\teol++;\n-\n-\t\tif (starts_with(buffer, \"author \"))\n-\t\t\tauthor = buffer + 7;\n-\t\tbuffer = eol;\n-\t}\n-\tif (!author) {\n+\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf oneline = STRBUF_INIT;\n+\tstruct pretty_print_context ctx = {0};\n+\n+\tctx.fmt = CMIT_FMT_USERFORMAT;\n+\tctx.abbrev = log->abbrev;\n+\tctx.subject = \"\";\n+\tctx.after_subject = \"\";\n+\tctx.date_mode.type = DATE_NORMAL;\n+\tctx.output_encoding = get_log_output_encoding();\n+\n+\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n+\t/* we can detect a total failure only by seeing \" <>\" in the output */\n+\tif (author.len <= 3) {\n \t\twarning(_(\"Missing author: %s\"),\n \t\t oid_to_hex(&commit->object.oid));\n-\t\treturn;\n-\t}\n-\tif (log->user_format) {\n-\t\tstruct pretty_print_context ctx = {0};\n-\t\tctx.fmt = CMIT_FMT_USERFORMAT;\n-\t\tctx.abbrev = log->abbrev;\n-\t\tctx.subject = \"\";\n-\t\tctx.after_subject = \"\";\n-\t\tctx.date_mode.type = DATE_NORMAL;\n-\t\tctx.output_encoding = get_log_output_encoding();\n-\t\tpretty_print_commit(&ctx, commit, &ufbuf);\n-\t\tbuffer = ufbuf.buf;\n-\t} else if (*buffer) {\n-\t\tbuffer++;\n+\t\tgoto out;\n \t}\n-\tinsert_one_record(log, author, !*buffer ? \"\" : buffer);\n-\tstrbuf_release(&ufbuf);\n-\tstrbuf_release(&buf);\n+\n+\tif (log->user_format)\n+\t\tpretty_print_commit(&ctx, commit, &oneline);\n+\telse\n+\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\n+\tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n+\n+out:\n+\tstrbuf_release(&author);\n+\tstrbuf_release(&oneline);\n }\n \n static void get_from_rev(struct rev_info *rev, struct shortlog *log)\n-- \n2.7.0.244.g0701a9d\n"},{"id":"276171","messageId":"20160115170933.GD21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 4/6] shortlog: optimize \"--summary\" mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:09:33Z","receivedAt":"2016-01-15T17:09:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If the user asked us only to show counts for each author,\nrather than the individual summary lines, then there is no\npoint in us generating the summaries only to throw them\naway. With this patch, I measured the following speedup for\n\"git shortlog -ns HEAD\" on linux.git (best-of-five):\n\n [before]\n real 0m5.644s\n user 0m5.472s\n sys 0m0.176s\n\n [after]\n real 0m5.257s\n user 0m5.104s\n sys 0m0.156s\n\nThat's only ~7%, but it's so easy to do, there's no good\nreason not to. We don't have to touch any downstream code,\nsince we already fill in the magic string \"\" to handle\ncommits without a message.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 256b868..3e1e0d6 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -152,10 +152,12 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\tgoto out;\n \t}\n \n-\tif (log->user_format)\n-\t\tpretty_print_commit(&ctx, commit, &oneline);\n-\telse\n-\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\tif (!log->summary) {\n+\t\tif (log->user_format)\n+\t\t\tpretty_print_commit(&ctx, commit, &oneline);\n+\t\telse\n+\t\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\t}\n \n \tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n \n-- \n2.7.0.244.g0701a9d\n"},{"id":"276172","messageId":"20160115171006.GE21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 5/6] shortlog: optimize out useless \"\" normalization","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:10:07Z","receivedAt":"2016-01-15T17:10:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are in --summary mode, we will always pass to\ninsert_one_record, which will then do some normalization\n(e.g., cutting out \"[PATCH]\"). There's no point in doing so\nif we aren't going to use the result anyway.\n\nThis drops my best-of-five for \"git shortlog -ns HEAD\" on\nlinux.git from:\n\n real 0m5.257s\n user 0m5.104s\n sys 0m0.156s\n\nto:\n\n real 0m5.194s\n user 0m5.028s\n sys 0m0.168s\n\nThat's only 1%, but arguably the result is clearer to read,\nas we're able to group our variable declarations inside the\nconditional block. It also opens up further optimization\npossibilities for future patches.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 63 +++++++++++++++++++++++++++++-------------------------\n 1 file changed, 34 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 3e1e0d6..c2bf9b2 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -31,13 +31,9 @@ static void insert_one_record(struct shortlog *log,\n \t\t\t const char *author,\n \t\t\t const char *oneline)\n {\n-\tconst char *dot3 = log->common_repo_prefix;\n-\tchar *buffer, *p;\n \tstruct string_list_item *item;\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n-\tconst char *eol;\n-\tstruct strbuf subject = STRBUF_INIT;\n \tstruct strbuf namemailbuf = STRBUF_INIT;\n \tstruct ident_split ident;\n \n@@ -59,34 +55,43 @@ static void insert_one_record(struct shortlog *log,\n \tif (item->util == NULL)\n \t\titem->util = xcalloc(1, sizeof(struct string_list));\n \n-\t/* Skip any leading whitespace, including any blank lines. */\n-\twhile (*oneline && isspace(*oneline))\n-\t\toneline++;\n-\teol = strchr(oneline, '\\n');\n-\tif (!eol)\n-\t\teol = oneline + strlen(oneline);\n-\tif (starts_with(oneline, \"[PATCH\")) {\n-\t\tchar *eob = strchr(oneline, ']');\n-\t\tif (eob && (!eol || eob < eol))\n-\t\t\toneline = eob + 1;\n-\t}\n-\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n-\t\toneline++;\n-\tformat_subject(&subject, oneline, \" \");\n-\tbuffer = strbuf_detach(&subject, NULL);\n-\n-\tif (dot3) {\n-\t\tint dot3len = strlen(dot3);\n-\t\tif (dot3len > 5) {\n-\t\t\twhile ((p = strstr(buffer, dot3)) != NULL) {\n-\t\t\t\tint taillen = strlen(p) - dot3len;\n-\t\t\t\tmemcpy(p, \"/.../\", 5);\n-\t\t\t\tmemmove(p + 5, p + dot3len, taillen + 1);\n+\tif (log->summary)\n+\t\tstring_list_append(item->util, xstrdup(\"\"));\n+\telse {\n+\t\tconst char *dot3 = log->common_repo_prefix;\n+\t\tchar *buffer, *p;\n+\t\tstruct strbuf subject = STRBUF_INIT;\n+\t\tconst char *eol;\n+\n+\t\t/* Skip any leading whitespace, including any blank lines. */\n+\t\twhile (*oneline && isspace(*oneline))\n+\t\t\toneline++;\n+\t\teol = strchr(oneline, '\\n');\n+\t\tif (!eol)\n+\t\t\teol = oneline + strlen(oneline);\n+\t\tif (starts_with(oneline, \"[PATCH\")) {\n+\t\t\tchar *eob = strchr(oneline, ']');\n+\t\t\tif (eob && (!eol || eob < eol))\n+\t\t\t\toneline = eob + 1;\n+\t\t}\n+\t\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n+\t\t\toneline++;\n+\t\tformat_subject(&subject, oneline, \" \");\n+\t\tbuffer = strbuf_detach(&subject, NULL);\n+\n+\t\tif (dot3) {\n+\t\t\tint dot3len = strlen(dot3);\n+\t\t\tif (dot3len > 5) {\n+\t\t\t\twhile ((p = strstr(buffer, dot3)) != NULL) {\n+\t\t\t\t\tint taillen = strlen(p) - dot3len;\n+\t\t\t\t\tmemcpy(p, \"/.../\", 5);\n+\t\t\t\t\tmemmove(p + 5, p + dot3len, taillen + 1);\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n-\t}\n \n-\tstring_list_append(item->util, buffer);\n+\t\tstring_list_append(item->util, buffer);\n+\t}\n }\n \n /*\n-- \n2.7.0.244.g0701a9d\n"},{"id":"276173","messageId":"20160115171048.GF21102@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"[PATCH 6/6] shortlog: optimize out useless string list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-15T17:10:49Z","receivedAt":"2016-01-15T17:10:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are in \"--summary\" mode, then we do not care about the\nactual list of subject onelines associated with each author.\nWe care only about the number. So rather than store a\nstring-list for each author full of \"\", let's just\nkeep a count.\n\nThis drops my best-of-five for \"git shortlog -ns HEAD\" on\nlinux.git from:\n\n real 0m5.194s\n user 0m5.028s\n sys 0m0.168s\n\nto:\n\n real 0m5.057s\n user 0m4.916s\n sys 0m0.144s\n\nThat's about 2.5%.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 43 +++++++++++++++++++++++++++++++------------\n 1 file changed, 31 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex c2bf9b2..3a0c863 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -14,7 +14,26 @@ static char const * const shortlog_usage[] = {\n \tNULL\n };\n \n-static int compare_by_number(const void *a1, const void *a2)\n+/*\n+ * The util field of our string_list_items will contain one of two things:\n+ *\n+ * - if --summary is not in use, it will point to a string list of the\n+ * oneline subjects assigned to this author\n+ *\n+ * - if --summary is in use, we don't need that list; we only need to know\n+ * its size. So we abuse the pointer slot to store our integer counter.\n+ *\n+ * This macro accesses the latter.\n+ */\n+#define UTIL_TO_INT(x) ((intptr_t)(x)->util)\n+\n+static int compare_by_counter(const void *a1, const void *a2)\n+{\n+\tconst struct string_list_item *i1 = a1, *i2 = a2;\n+\treturn UTIL_TO_INT(i2) - UTIL_TO_INT(i1);\n+}\n+\n+static int compare_by_list(const void *a1, const void *a2)\n {\n \tconst struct string_list_item *i1 = a1, *i2 = a2;\n \tconst struct string_list *l1 = i1->util, *l2 = i2->util;\n@@ -52,11 +71,9 @@ static void insert_one_record(struct shortlog *log,\n \t\tstrbuf_addf(&namemailbuf, \" <%.*s>\", (int)maillen, mailbuf);\n \n \titem = string_list_insert(&log->list, namemailbuf.buf);\n-\tif (item->util == NULL)\n-\t\titem->util = xcalloc(1, sizeof(struct string_list));\n \n \tif (log->summary)\n-\t\tstring_list_append(item->util, xstrdup(\"\"));\n+\t\titem->util = (void *)(UTIL_TO_INT(item) + 1);\n \telse {\n \t\tconst char *dot3 = log->common_repo_prefix;\n \t\tchar *buffer, *p;\n@@ -90,6 +107,8 @@ static void insert_one_record(struct shortlog *log,\n \t\t\t}\n \t\t}\n \n+\t\tif (item->util == NULL)\n+\t\t\titem->util = xcalloc(1, sizeof(struct string_list));\n \t\tstring_list_append(item->util, buffer);\n \t}\n }\n@@ -315,14 +334,14 @@ void shortlog_output(struct shortlog *log)\n \n \tif (log->sort_by_number)\n \t\tqsort(log->list.items, log->list.nr, sizeof(struct string_list_item),\n-\t\t\tcompare_by_number);\n+\t\t log->summary ? compare_by_counter : compare_by_list);\n \tfor (i = 0; i < log->list.nr; i++) {\n-\t\tstruct string_list *onelines = log->list.items[i].util;\n-\n+\t\tconst struct string_list_item *item = &log->list.items[i];\n \t\tif (log->summary) {\n-\t\t\tprintf(\"%6d\\t%s\\n\", onelines->nr, log->list.items[i].string);\n+\t\t\tprintf(\"%6d\\t%s\\n\", (int)UTIL_TO_INT(item), item->string);\n \t\t} else {\n-\t\t\tprintf(\"%s (%d):\\n\", log->list.items[i].string, onelines->nr);\n+\t\t\tstruct string_list *onelines = item->util;\n+\t\t\tprintf(\"%s (%d):\\n\", item->string, onelines->nr);\n \t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n \t\t\t\tconst char *msg = onelines->items[j].string;\n \n@@ -335,11 +354,11 @@ void shortlog_output(struct shortlog *log)\n \t\t\t\t\tprintf(\" %s\\n\", msg);\n \t\t\t}\n \t\t\tputchar('\\n');\n+\t\t\tonelines->strdup_strings = 1;\n+\t\t\tstring_list_clear(onelines, 0);\n+\t\t\tfree(onelines);\n \t\t}\n \n-\t\tonelines->strdup_strings = 1;\n-\t\tstring_list_clear(onelines, 0);\n-\t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n \n-- \n2.7.0.244.g0701a9d\n"},{"id":"276211","messageId":"xmqqpox2fq7t.fsf@gitster.mtv.corp.google.com","threadId":"41201","inReplyTo":"20160115170627.GA20983@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] shortlog fixes and optimizations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-15T22:11:18Z","receivedAt":"2016-01-15T22:11:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> These are split out from my the shortlog-trailer series I sent a few\n> weeks ago. The \"trailer\" parts still need some re-working, but there is\n> no need to hold these fixes hostage.\n>\n> I also dropped the early part of the series, adding skip_prefix_icase().\n> After digging into the history, I ended up reworking the first patch\n> here to do a more thorough parse, so we don't need it anymore.\n>\n> Thanks to Eric Sunshine for review on the first iteration; this\n> incorporates his comments.\n>\n> [1/6]: shortlog: match both \"Author:\" and \"author\" on stdin\n> [2/6]: shortlog: use strbufs to read from stdin\n> [3/6]: shortlog: replace hand-parsing of author with pretty-printer\n> [4/6]: shortlog: optimize \"--summary\" mode\n> [5/6]: shortlog: optimize out useless \"\" normalization\n> [6/6]: shortlog: optimize out useless string list\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/283091\n\nThanks for resurrecting these patches. They all look good.\n\n[PATCH 2/6] is a bit unfortunate timing-wise, but I think we can\nmanage ;-).\n"},{"id":"276217","messageId":"CAPig+cQXDn+iR29vKzSRAUBYgXzTiR5MykKAtvCcF0SE7J4kUg@mail.gmail.com","threadId":"41201","inReplyTo":"20160115170823.GA21102@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-15T23:19:30Z","receivedAt":"2016-01-15T23:19:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Friday, January 15, 2016, Jeff King wrote:\n> The original git-shortlog could read both the normal \"git\n> log\" output as well as \"git log --format=raw\". However, when it was\n> converted to C by b8ec592 (Build in shortlog, 2006-10-22),\n> the trailing colon became mandatory, and we no longer\n> matched the raw output.\n>\n> Given the amount of intervening time without any bug\n> reports, it's probable that nobody cares. But given that\n> it's easy to fix, and that the end result is hopefully more\n> obvious and flexible (it could now easily accomodate matching\n> \"Committer\"), let's just make it work.\n>\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> @@ -89,13 +89,34 @@ static void insert_one_record(struct shortlog *log,\n> +/*\n> + * If header is \"author\", match candidate against the regex /[Aa]uthor:? /,\n> + * and return a pointer to the remainder of the string in out_value.\n> + */\n> +static int match_ident_header(const char *candidate, const char *header,\n> + const char **out_value)\n> +{\n> + const char *v;\n> +\n> + if (tolower(*candidate++) != tolower(*header++))\n> + return 0;\n\nPresumably, this will never be invoked as match_ident_header(\"\", \"\",\n...) so we don't have to worry about it accessing beyond end-of-string\nwhen it gets past this conditional. Does it deserve an\nassert(*candidate) at the top of the function, though, or is that\noverkill?\n\n> + if (!skip_prefix(candidate, header, &v))\n> + return 0;\n> + if (*v == ':')\n> + v++;\n> + if (*v++ != ' ')\n> + return 0;\n> + *out_value = v;\n> + return 1;\n> +}\n"},{"id":"276290","messageId":"20160118192621.GA981@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160115170823.GA21102@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T19:26:21Z","receivedAt":"2016-01-18T19:26:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 15, 2016 at 12:08:23PM -0500, Jeff King wrote:\n\n> The original git-shortlog could read both the normal \"git\n> log\" output as well as \"git log --format=raw\". However, when it was\n> converted to C by b8ec592 (Build in shortlog, 2006-10-22),\n> the trailing colon became mandatory, and we no longer\n> matched the raw output.\n> \n> Given the amount of intervening time without any bug\n> reports, it's probable that nobody cares. But given that\n> it's easy to fix, and that the end result is hopefully more\n> obvious and flexible (it could now easily accomodate matching\n> \"Committer\"), let's just make it work.\n\nI rebased the rest of my shortlog-trailer series on this, and sadly,\nthis final sentence isn't quite true.\n\nThe regular \"git log\" output uses \"Commit:\" for the committer line, and\nthe raw output uses \"committer\". So the match_ident_header function\n_can't_ be reused.\n\nSo it's not wrong, but it's perhaps more complicated than it needs to\nbe. We could scrap this patch in favor of just:\n\n if (!skip_prefix(author, \"Author: \", &v) &&\n !skip_prefix(author, \"author \", &v))\n continue;\n\nThat is technically more strict (it does not take \"author: \", which is\naccepted by the current code), but matches \"git log\" and \"git log --raw\"\noutput, and misses nothing that git has ever generated. And it extends\nnaturally to:\n\n if (!skip_prefix(author, \"Commit: \", &v) &&\n !skip_prefix(author, \"committer \", &v))\n continue;\n\n-Peff\n"},{"id":"276291","messageId":"20160118192739.GA1009@sigill.intra.peff.net","threadId":"41201","inReplyTo":"CAPig+cQXDn+iR29vKzSRAUBYgXzTiR5MykKAtvCcF0SE7J4kUg@mail.gmail.com","subject":"Re: [PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T19:27:39Z","receivedAt":"2016-01-18T19:27:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 15, 2016 at 06:19:30PM -0500, Eric Sunshine wrote:\n\n> > +/*\n> > + * If header is \"author\", match candidate against the regex /[Aa]uthor:? /,\n> > + * and return a pointer to the remainder of the string in out_value.\n> > + */\n> > +static int match_ident_header(const char *candidate, const char *header,\n> > + const char **out_value)\n> > +{\n> > + const char *v;\n> > +\n> > + if (tolower(*candidate++) != tolower(*header++))\n> > + return 0;\n> \n> Presumably, this will never be invoked as match_ident_header(\"\", \"\",\n> ...) so we don't have to worry about it accessing beyond end-of-string\n> when it gets past this conditional. Does it deserve an\n> assert(*candidate) at the top of the function, though, or is that\n> overkill?\n\nGood point. It shouldn't happen (we will always feed a string literal),\nbut it never hurts to document assumptions with an assertion.\n\nHowever, there is some reason to think this isn't the ideal function;\nsee the message I just posted elsewhere in the thread.\n\n-Peff\n"},{"id":"276297","messageId":"xmqqwpr6d5nd.fsf@gitster.mtv.corp.google.com","threadId":"41201","inReplyTo":"20160118192621.GA981@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-18T19:55:18Z","receivedAt":"2016-01-18T19:55:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So it's not wrong, but it's perhaps more complicated than it needs to\n> be. We could scrap this patch in favor of just:\n>\n> if (!skip_prefix(author, \"Author: \", &v) &&\n> !skip_prefix(author, \"author \", &v))\n> continue;\n>\n> That is technically more strict (it does not take \"author: \", which is\n> accepted by the current code), but matches \"git log\" and \"git log --raw\"\n> output, and misses nothing that git has ever generated. And it extends\n> naturally to:\n>\n> if (!skip_prefix(author, \"Commit: \", &v) &&\n> !skip_prefix(author, \"committer \", &v))\n> continue;\n\nYeah, I agree that the above long-hand would be more readable.\n"},{"id":"276299","messageId":"20160118200136.GA9514@sigill.intra.peff.net","threadId":"41201","inReplyTo":"xmqqwpr6d5nd.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 0/6] shortlog fixes and optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:01:36Z","receivedAt":"2016-01-18T20:01:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 18, 2016 at 11:55:18AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So it's not wrong, but it's perhaps more complicated than it needs to\n> > be. We could scrap this patch in favor of just:\n> >\n> > if (!skip_prefix(author, \"Author: \", &v) &&\n> > !skip_prefix(author, \"author \", &v))\n> > continue;\n> >\n> > That is technically more strict (it does not take \"author: \", which is\n> > accepted by the current code), but matches \"git log\" and \"git log --raw\"\n> > output, and misses nothing that git has ever generated. And it extends\n> > naturally to:\n> >\n> > if (!skip_prefix(author, \"Commit: \", &v) &&\n> > !skip_prefix(author, \"committer \", &v))\n> > continue;\n> \n> Yeah, I agree that the above long-hand would be more readable.\n\nOK. Here it is again (the whole series, since the change creates minor\nconflicts some of the later patches).\n\nThe interdiff is:\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 3a0c863..adbf1fd 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -113,27 +113,6 @@ static void insert_one_record(struct shortlog *log,\n \t}\n }\n \n-/*\n- * If header is \"author\", match candidate against the regex /[Aa]uthor:? /,\n- * and return a pointer to the remainder of the string in out_value.\n- */\n-static int match_ident_header(const char *candidate, const char *header,\n-\t\t\t const char **out_value)\n-{\n-\tconst char *v;\n-\n-\tif (tolower(*candidate++) != tolower(*header++))\n-\t\treturn 0;\n-\tif (!skip_prefix(candidate, header, &v))\n-\t\treturn 0;\n-\tif (*v == ':')\n-\t\tv++;\n-\tif (*v++ != ' ')\n-\t\treturn 0;\n-\t*out_value = v;\n-\treturn 1;\n-}\n-\n static void read_from_stdin(struct shortlog *log)\n {\n \tstruct strbuf author = STRBUF_INIT;\n@@ -141,7 +120,8 @@ static void read_from_stdin(struct shortlog *log)\n \n \twhile (strbuf_getline(&author, stdin, '\\n') != EOF) {\n \t\tconst char *v;\n-\t\tif (!match_ident_header(author.buf, \"author\", &v))\n+\t\tif (!skip_prefix(author.buf, \"Author: \", &v) &&\n+\t\t !skip_prefix(author.buf, \"author \", &v))\n \t\t\tcontinue;\n \t\twhile (strbuf_getline(&oneline, stdin, '\\n') != EOF &&\n \t\t oneline.len)\n"},{"id":"276300","messageId":"20160118200240.GA15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 1/6] shortlog: match both \"Author:\" and \"author\" on stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:40Z","receivedAt":"2016-01-18T20:02:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The original git-shortlog could read both the normal \"git\nlog\" output as well as \"git log --format=raw\". However, when\nit was converted to C by b8ec592 (Build in shortlog,\n2006-10-22), the trailing colon became mandatory, and we no\nlonger matched the raw output.\n\nGiven the amount of intervening time without any bug\nreports, it's probable that nobody cares. But it's\nrelatively easy to fix, and the end result is hopefully more\nreadable than the original.\n\nNote that this no longer matches \"author: \", which we did\nbefore, but that has never been a format generated by git.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 7 ++++---\n t/t4201-shortlog.sh | 6 ++++++\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 35ebd17..ab25b44 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -94,8 +94,9 @@ static void read_from_stdin(struct shortlog *log)\n \tchar author[1024], oneline[1024];\n \n \twhile (fgets(author, sizeof(author), stdin) != NULL) {\n-\t\tif (!(author[0] == 'A' || author[0] == 'a') ||\n-\t\t !starts_with(author + 1, \"uthor: \"))\n+\t\tconst char *v;\n+\t\tif (!skip_prefix(author, \"Author: \", &v) &&\n+\t\t !skip_prefix(author, \"author \", &v))\n \t\t\tcontinue;\n \t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n \t\t oneline[0] != '\\n')\n@@ -103,7 +104,7 @@ static void read_from_stdin(struct shortlog *log)\n \t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n \t\t oneline[0] == '\\n')\n \t\t\t; /* discard blanks */\n-\t\tinsert_one_record(log, author + 8, oneline);\n+\t\tinsert_one_record(log, v, oneline);\n \t}\n }\n \ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 7600a3e..82b2314 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -120,6 +120,12 @@ test_expect_success !MINGW 'shortlog from non-git directory' '\n \ttest_cmp expect out\n '\n \n+test_expect_success !MINGW 'shortlog can read --format=raw output' '\n+\tgit log --format=raw HEAD >log &&\n+\tGIT_DIR=non-existing git shortlog -w out &&\n+\ttest_cmp expect out\n+'\n+\n test_expect_success 'shortlog should add newline when input line matches wraplen' '\n \tcat >expect <<\\EOF &&\n A U Thor (2):\n-- \n2.7.0.248.g5eafd77\n"},{"id":"276302","messageId":"20160118200244.GB15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 2/6] shortlog: use strbufs to read from stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:44Z","receivedAt":"2016-01-18T20:02:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We currently use fixed-size buffers with fgets(), which\ncould lead to incorrect results in the unlikely event that a\nline had something like \"Author:\" at exactly its 1024th\ncharacter.\n\nBut it's easy to convert this to a strbuf, and because we\ncan reuse the same buffer through the loop, we don't even\npay the extra allocation cost.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 21 ++++++++++++---------\n 1 file changed, 12 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex ab25b44..6c0a72e 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -91,21 +91,24 @@ static void insert_one_record(struct shortlog *log,\n \n static void read_from_stdin(struct shortlog *log)\n {\n-\tchar author[1024], oneline[1024];\n+\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf oneline = STRBUF_INIT;\n \n-\twhile (fgets(author, sizeof(author), stdin) != NULL) {\n+\twhile (strbuf_getline(&author, stdin, '\\n') != EOF) {\n \t\tconst char *v;\n-\t\tif (!skip_prefix(author, \"Author: \", &v) &&\n-\t\t !skip_prefix(author, \"author \", &v))\n+\t\tif (!skip_prefix(author.buf, \"Author: \", &v) &&\n+\t\t !skip_prefix(author.buf, \"author \", &v))\n \t\t\tcontinue;\n-\t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n-\t\t oneline[0] != '\\n')\n+\t\twhile (strbuf_getline(&oneline, stdin, '\\n') != EOF &&\n+\t\t oneline.len)\n \t\t\t; /* discard headers */\n-\t\twhile (fgets(oneline, sizeof(oneline), stdin) &&\n-\t\t oneline[0] == '\\n')\n+\t\twhile (strbuf_getline(&oneline, stdin, '\\n') != EOF &&\n+\t\t !oneline.len)\n \t\t\t; /* discard blanks */\n-\t\tinsert_one_record(log, v, oneline);\n+\t\tinsert_one_record(log, v, oneline.buf);\n \t}\n+\tstrbuf_release(&author);\n+\tstrbuf_release(&oneline);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n-- \n2.7.0.248.g5eafd77\n"},{"id":"276301","messageId":"20160118200248.GC15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:48Z","receivedAt":"2016-01-18T20:02:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When gathering the author and oneline subject for each\ncommit, we hand-parse the commit headers to find the\n\"author\" line, and then continue past to the blank line at\nthe end of the header.\n\nWe can replace this tricky hand-parsing by simply asking the\npretty-printer for the relevant items. This also decouples\nthe author and oneline parsing, opening up some new\noptimizations in further commits.\n\nOne reason to avoid the pretty-printer is that it might be\nless efficient than hand-parsing. However, I measured no\nslowdown at all running \"git shortlog -ns HEAD\" on\nlinux.git.\n\nAs a bonus, we also fix a memory leak in the (uncommon) case\nthat the author field is blank.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 62 +++++++++++++++++++++++-------------------------------\n 1 file changed, 26 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 6c0a72e..1261ec4 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -113,45 +113,35 @@ static void read_from_stdin(struct shortlog *log)\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n-\tconst char *author = NULL, *buffer;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tstruct strbuf ufbuf = STRBUF_INIT;\n-\n-\tpp_commit_easy(CMIT_FMT_RAW, commit, &buf);\n-\tbuffer = buf.buf;\n-\twhile (*buffer && *buffer != '\\n') {\n-\t\tconst char *eol = strchr(buffer, '\\n');\n-\n-\t\tif (eol == NULL)\n-\t\t\teol = buffer + strlen(buffer);\n-\t\telse\n-\t\t\teol++;\n-\n-\t\tif (starts_with(buffer, \"author \"))\n-\t\t\tauthor = buffer + 7;\n-\t\tbuffer = eol;\n-\t}\n-\tif (!author) {\n+\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf oneline = STRBUF_INIT;\n+\tstruct pretty_print_context ctx = {0};\n+\n+\tctx.fmt = CMIT_FMT_USERFORMAT;\n+\tctx.abbrev = log->abbrev;\n+\tctx.subject = \"\";\n+\tctx.after_subject = \"\";\n+\tctx.date_mode.type = DATE_NORMAL;\n+\tctx.output_encoding = get_log_output_encoding();\n+\n+\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n+\t/* we can detect a total failure only by seeing \" <>\" in the output */\n+\tif (author.len <= 3) {\n \t\twarning(_(\"Missing author: %s\"),\n \t\t oid_to_hex(&commit->object.oid));\n-\t\treturn;\n-\t}\n-\tif (log->user_format) {\n-\t\tstruct pretty_print_context ctx = {0};\n-\t\tctx.fmt = CMIT_FMT_USERFORMAT;\n-\t\tctx.abbrev = log->abbrev;\n-\t\tctx.subject = \"\";\n-\t\tctx.after_subject = \"\";\n-\t\tctx.date_mode.type = DATE_NORMAL;\n-\t\tctx.output_encoding = get_log_output_encoding();\n-\t\tpretty_print_commit(&ctx, commit, &ufbuf);\n-\t\tbuffer = ufbuf.buf;\n-\t} else if (*buffer) {\n-\t\tbuffer++;\n+\t\tgoto out;\n \t}\n-\tinsert_one_record(log, author, !*buffer ? \"\" : buffer);\n-\tstrbuf_release(&ufbuf);\n-\tstrbuf_release(&buf);\n+\n+\tif (log->user_format)\n+\t\tpretty_print_commit(&ctx, commit, &oneline);\n+\telse\n+\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\n+\tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n+\n+out:\n+\tstrbuf_release(&author);\n+\tstrbuf_release(&oneline);\n }\n \n static void get_from_rev(struct rev_info *rev, struct shortlog *log)\n-- \n2.7.0.248.g5eafd77\n"},{"id":"276304","messageId":"20160118200251.GD15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 4/6] shortlog: optimize \"--summary\" mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:52Z","receivedAt":"2016-01-18T20:02:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If the user asked us only to show counts for each author,\nrather than the individual summary lines, then there is no\npoint in us generating the summaries only to throw them\naway. With this patch, I measured the following speedup for\n\"git shortlog -ns HEAD\" on linux.git (best-of-five):\n\n [before]\n real 0m5.644s\n user 0m5.472s\n sys 0m0.176s\n\n [after]\n real 0m5.257s\n user 0m5.104s\n sys 0m0.156s\n\nThat's only ~7%, but it's so easy to do, there's no good\nreason not to. We don't have to touch any downstream code,\nsince we already fill in the magic string \"\" to handle\ncommits without a message.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 1261ec4..973b50d 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -132,10 +132,12 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\tgoto out;\n \t}\n \n-\tif (log->user_format)\n-\t\tpretty_print_commit(&ctx, commit, &oneline);\n-\telse\n-\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\tif (!log->summary) {\n+\t\tif (log->user_format)\n+\t\t\tpretty_print_commit(&ctx, commit, &oneline);\n+\t\telse\n+\t\t\tformat_commit_message(commit, \"%s\", &oneline, &ctx);\n+\t}\n \n \tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n \n-- \n2.7.0.248.g5eafd77\n"},{"id":"276303","messageId":"20160118200255.GE15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 5/6] shortlog: optimize out useless \"\" normalization","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:56Z","receivedAt":"2016-01-18T20:02:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are in --summary mode, we will always pass to\ninsert_one_record, which will then do some normalization\n(e.g., cutting out \"[PATCH]\"). There's no point in doing so\nif we aren't going to use the result anyway.\n\nThis drops my best-of-five for \"git shortlog -ns HEAD\" on\nlinux.git from:\n\n real 0m5.257s\n user 0m5.104s\n sys 0m0.156s\n\nto:\n\n real 0m5.194s\n user 0m5.028s\n sys 0m0.168s\n\nThat's only 1%, but arguably the result is clearer to read,\nas we're able to group our variable declarations inside the\nconditional block. It also opens up further optimization\npossibilities for future patches.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 63 +++++++++++++++++++++++++++++-------------------------\n 1 file changed, 34 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 973b50d..a7708c3 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -31,13 +31,9 @@ static void insert_one_record(struct shortlog *log,\n \t\t\t const char *author,\n \t\t\t const char *oneline)\n {\n-\tconst char *dot3 = log->common_repo_prefix;\n-\tchar *buffer, *p;\n \tstruct string_list_item *item;\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n-\tconst char *eol;\n-\tstruct strbuf subject = STRBUF_INIT;\n \tstruct strbuf namemailbuf = STRBUF_INIT;\n \tstruct ident_split ident;\n \n@@ -59,34 +55,43 @@ static void insert_one_record(struct shortlog *log,\n \tif (item->util == NULL)\n \t\titem->util = xcalloc(1, sizeof(struct string_list));\n \n-\t/* Skip any leading whitespace, including any blank lines. */\n-\twhile (*oneline && isspace(*oneline))\n-\t\toneline++;\n-\teol = strchr(oneline, '\\n');\n-\tif (!eol)\n-\t\teol = oneline + strlen(oneline);\n-\tif (starts_with(oneline, \"[PATCH\")) {\n-\t\tchar *eob = strchr(oneline, ']');\n-\t\tif (eob && (!eol || eob < eol))\n-\t\t\toneline = eob + 1;\n-\t}\n-\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n-\t\toneline++;\n-\tformat_subject(&subject, oneline, \" \");\n-\tbuffer = strbuf_detach(&subject, NULL);\n-\n-\tif (dot3) {\n-\t\tint dot3len = strlen(dot3);\n-\t\tif (dot3len > 5) {\n-\t\t\twhile ((p = strstr(buffer, dot3)) != NULL) {\n-\t\t\t\tint taillen = strlen(p) - dot3len;\n-\t\t\t\tmemcpy(p, \"/.../\", 5);\n-\t\t\t\tmemmove(p + 5, p + dot3len, taillen + 1);\n+\tif (log->summary)\n+\t\tstring_list_append(item->util, xstrdup(\"\"));\n+\telse {\n+\t\tconst char *dot3 = log->common_repo_prefix;\n+\t\tchar *buffer, *p;\n+\t\tstruct strbuf subject = STRBUF_INIT;\n+\t\tconst char *eol;\n+\n+\t\t/* Skip any leading whitespace, including any blank lines. */\n+\t\twhile (*oneline && isspace(*oneline))\n+\t\t\toneline++;\n+\t\teol = strchr(oneline, '\\n');\n+\t\tif (!eol)\n+\t\t\teol = oneline + strlen(oneline);\n+\t\tif (starts_with(oneline, \"[PATCH\")) {\n+\t\t\tchar *eob = strchr(oneline, ']');\n+\t\t\tif (eob && (!eol || eob < eol))\n+\t\t\t\toneline = eob + 1;\n+\t\t}\n+\t\twhile (*oneline && isspace(*oneline) && *oneline != '\\n')\n+\t\t\toneline++;\n+\t\tformat_subject(&subject, oneline, \" \");\n+\t\tbuffer = strbuf_detach(&subject, NULL);\n+\n+\t\tif (dot3) {\n+\t\t\tint dot3len = strlen(dot3);\n+\t\t\tif (dot3len > 5) {\n+\t\t\t\twhile ((p = strstr(buffer, dot3)) != NULL) {\n+\t\t\t\t\tint taillen = strlen(p) - dot3len;\n+\t\t\t\t\tmemcpy(p, \"/.../\", 5);\n+\t\t\t\t\tmemmove(p + 5, p + dot3len, taillen + 1);\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n-\t}\n \n-\tstring_list_append(item->util, buffer);\n+\t\tstring_list_append(item->util, buffer);\n+\t}\n }\n \n static void read_from_stdin(struct shortlog *log)\n-- \n2.7.0.248.g5eafd77\n"},{"id":"276305","messageId":"20160118200259.GF15836@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200136.GA9514@sigill.intra.peff.net","subject":"[PATCH 6/6] shortlog: optimize out useless string list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:02:59Z","receivedAt":"2016-01-18T20:02:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are in \"--summary\" mode, then we do not care about the\nactual list of subject onelines associated with each author.\nWe care only about the number. So rather than store a\nstring-list for each author full of \"\", let's just\nkeep a count.\n\nThis drops my best-of-five for \"git shortlog -ns HEAD\" on\nlinux.git from:\n\n real 0m5.194s\n user 0m5.028s\n sys 0m0.168s\n\nto:\n\n real 0m5.057s\n user 0m4.916s\n sys 0m0.144s\n\nThat's about 2.5%.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 43 +++++++++++++++++++++++++++++++------------\n 1 file changed, 31 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex a7708c3..adbf1fd 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -14,7 +14,26 @@ static char const * const shortlog_usage[] = {\n \tNULL\n };\n \n-static int compare_by_number(const void *a1, const void *a2)\n+/*\n+ * The util field of our string_list_items will contain one of two things:\n+ *\n+ * - if --summary is not in use, it will point to a string list of the\n+ * oneline subjects assigned to this author\n+ *\n+ * - if --summary is in use, we don't need that list; we only need to know\n+ * its size. So we abuse the pointer slot to store our integer counter.\n+ *\n+ * This macro accesses the latter.\n+ */\n+#define UTIL_TO_INT(x) ((intptr_t)(x)->util)\n+\n+static int compare_by_counter(const void *a1, const void *a2)\n+{\n+\tconst struct string_list_item *i1 = a1, *i2 = a2;\n+\treturn UTIL_TO_INT(i2) - UTIL_TO_INT(i1);\n+}\n+\n+static int compare_by_list(const void *a1, const void *a2)\n {\n \tconst struct string_list_item *i1 = a1, *i2 = a2;\n \tconst struct string_list *l1 = i1->util, *l2 = i2->util;\n@@ -52,11 +71,9 @@ static void insert_one_record(struct shortlog *log,\n \t\tstrbuf_addf(&namemailbuf, \" <%.*s>\", (int)maillen, mailbuf);\n \n \titem = string_list_insert(&log->list, namemailbuf.buf);\n-\tif (item->util == NULL)\n-\t\titem->util = xcalloc(1, sizeof(struct string_list));\n \n \tif (log->summary)\n-\t\tstring_list_append(item->util, xstrdup(\"\"));\n+\t\titem->util = (void *)(UTIL_TO_INT(item) + 1);\n \telse {\n \t\tconst char *dot3 = log->common_repo_prefix;\n \t\tchar *buffer, *p;\n@@ -90,6 +107,8 @@ static void insert_one_record(struct shortlog *log,\n \t\t\t}\n \t\t}\n \n+\t\tif (item->util == NULL)\n+\t\t\titem->util = xcalloc(1, sizeof(struct string_list));\n \t\tstring_list_append(item->util, buffer);\n \t}\n }\n@@ -295,14 +314,14 @@ void shortlog_output(struct shortlog *log)\n \n \tif (log->sort_by_number)\n \t\tqsort(log->list.items, log->list.nr, sizeof(struct string_list_item),\n-\t\t\tcompare_by_number);\n+\t\t log->summary ? compare_by_counter : compare_by_list);\n \tfor (i = 0; i < log->list.nr; i++) {\n-\t\tstruct string_list *onelines = log->list.items[i].util;\n-\n+\t\tconst struct string_list_item *item = &log->list.items[i];\n \t\tif (log->summary) {\n-\t\t\tprintf(\"%6d\\t%s\\n\", onelines->nr, log->list.items[i].string);\n+\t\t\tprintf(\"%6d\\t%s\\n\", (int)UTIL_TO_INT(item), item->string);\n \t\t} else {\n-\t\t\tprintf(\"%s (%d):\\n\", log->list.items[i].string, onelines->nr);\n+\t\t\tstruct string_list *onelines = item->util;\n+\t\t\tprintf(\"%s (%d):\\n\", item->string, onelines->nr);\n \t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n \t\t\t\tconst char *msg = onelines->items[j].string;\n \n@@ -315,11 +334,11 @@ void shortlog_output(struct shortlog *log)\n \t\t\t\t\tprintf(\" %s\\n\", msg);\n \t\t\t}\n \t\t\tputchar('\\n');\n+\t\t\tonelines->strdup_strings = 1;\n+\t\t\tstring_list_clear(onelines, 0);\n+\t\t\tfree(onelines);\n \t\t}\n \n-\t\tonelines->strdup_strings = 1;\n-\t\tstring_list_clear(onelines, 0);\n-\t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n \n-- \n2.7.0.248.g5eafd77\n"},{"id":"276307","messageId":"20160118201337.GA15943@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118200248.GC15836@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T20:13:38Z","receivedAt":"2016-01-18T20:13:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 18, 2016 at 03:02:48PM -0500, Jeff King wrote:\n\n> +\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n> +\t/* we can detect a total failure only by seeing \" <>\" in the output */\n> +\tif (author.len <= 3) {\n> \t\twarning(_(\"Missing author: %s\"),\n> \t\t oid_to_hex(&commit->object.oid));\n> [...]\n> +\t\tgoto out;\n> \t}\n\nOne note on this. In the linux.git tree, this warning actually triggers,\nbecause there is a commit with a bogus empty author:\n\n $ git log -1 --format=raw af25e94d4dc | grep ^author\n author <> 1120285620 -0700\n\nWhereas in the original code, you really do get a line with a blank\nname.\n\nI think what the new code does is quite reasonable, but I'm not sure if:\n\n 1. People really want a syntactically valid empty name to be\n represented.\n\nand\n\n 2. Regardless of the output, if kernel folks will be annoyed by the\n warning whenever they run a full-repo shortlog.\n\n-Peff\n"},{"id":"276321","messageId":"20160118230435.GA16898@sigill.intra.peff.net","threadId":"41201","inReplyTo":"20160118201337.GA15943@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-18T23:04:35Z","receivedAt":"2016-01-18T23:04:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 18, 2016 at 03:13:37PM -0500, Jeff King wrote:\n\n> On Mon, Jan 18, 2016 at 03:02:48PM -0500, Jeff King wrote:\n> \n> > +\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n> > +\t/* we can detect a total failure only by seeing \" <>\" in the output */\n> > +\tif (author.len <= 3) {\n> > \t\twarning(_(\"Missing author: %s\"),\n> > \t\t oid_to_hex(&commit->object.oid));\n> > [...]\n> > +\t\tgoto out;\n> > \t}\n> \n> One note on this. In the linux.git tree, this warning actually triggers,\n> because there is a commit with a bogus empty author:\n> \n> $ git log -1 --format=raw af25e94d4dc | grep ^author\n> author <> 1120285620 -0700\n> \n> Whereas in the original code, you really do get a line with a blank\n> name.\n> \n> I think what the new code does is quite reasonable, but I'm not sure if:\n> \n> 1. People really want a syntactically valid empty name to be\n> represented.\n> \n> and\n> \n> 2. Regardless of the output, if kernel folks will be annoyed by the\n> warning whenever they run a full-repo shortlog.\n\nAfter thinking on this, I'm in favor of removing this warning entirely.\nMy reasons are given in the commit message below, which can apply on top\nof the series. It could also be squashed in to 2/6, but given that it\nis removing the test added by cd4f09e (shortlog: ignore commits with\nmissing authors, 2013-09-18), I think it's worth recording as a separate\ncommit.\n\n-- >8 --\nSubject: [PATCH] shortlog: don't warn on empty author\n\nGit tries to avoid creating a commit with an empty author\nname or email. However, commits created by older, less\nstrict versions of git may still be in the history. There's\nnot much point in issuing a warning to stderr for an empty\nauthor. The user can't do anything about it now, and we are\nbetter off to simply include it in the shortlog output as an\nempty name/email, and let the caller process it however they\nsee fit.\n\nOlder versions of shortlog differentiated between \"author\nheader not present\" (which complained) and \"author\nname/email are blank\" (which included the empty ident in the\noutput). But since switching to format_commit_message, we\ncomplain to stderr about either case (linux.git has a blank\nauthor deep in its history which triggers this).\n\nWe could try to restore the older behavior (complaining only\nabout the missing header), but in retrospect, there's not\nmuch point in differentiating these cases. A missing\nauthor header is bogus, but as for the \"blank\" case, the\nonly useful behavior is to add it to the \"empty name\"\ncollection.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 8 --------\n t/t4201-shortlog.sh | 16 ----------------\n 2 files changed, 24 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex adbf1fd..e32be39 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -149,13 +149,6 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tctx.output_encoding = get_log_output_encoding();\n \n \tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n-\t/* we can detect a total failure only by seeing \" <>\" in the output */\n-\tif (author.len <= 3) {\n-\t\twarning(_(\"Missing author: %s\"),\n-\t\t oid_to_hex(&commit->object.oid));\n-\t\tgoto out;\n-\t}\n-\n \tif (!log->summary) {\n \t\tif (log->user_format)\n \t\t\tpretty_print_commit(&ctx, commit, &oneline);\n@@ -165,7 +158,6 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \n \tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n \n-out:\n \tstrbuf_release(&author);\n \tstrbuf_release(&oneline);\n }\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 82b2314..f5e6367 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -178,22 +178,6 @@ test_expect_success !MINGW 'shortlog encoding' '\n \tgit shortlog HEAD~2.. > out &&\n test_cmp expect out'\n \n-test_expect_success 'shortlog ignores commits with missing authors' '\n-\tgit commit --allow-empty -m normal &&\n-\tgit commit --allow-empty -m soon-to-be-broken &&\n-\tgit cat-file commit HEAD >commit.tmp &&\n-\tsed \"/^author/d\" commit.tmp >broken.tmp &&\n-\tcommit=$(git hash-object -w -t commit --stdin expect <<-\\EOF &&\n-\tA U Thor (1):\n-\t normal\n-\n-\tEOF\n-\tgit shortlog HEAD~2.. >actual &&\n-\ttest_cmp expect actual\n-'\n-\n test_expect_success 'shortlog with revision pseudo options' '\n \tgit shortlog --all &&\n \tgit shortlog --branches &&\n-- \n2.7.0.248.g5eafd77\n"},{"id":"276330","messageId":"xmqqegdecjrp.fsf@gitster.mtv.corp.google.com","threadId":"41201","inReplyTo":"20160118230435.GA16898@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-19T03:47:54Z","receivedAt":"2016-01-19T03:47:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> After thinking on this, I'm in favor of removing this warning entirely.\n> My reasons are given in the commit message below, which can apply on top\n> of the series. It could also be squashed in to 2/6, but given that it\n> is removing the test added by cd4f09e (shortlog: ignore commits with\n> missing authors, 2013-09-18), I think it's worth recording as a separate\n> commit.\n\nI agree 100% with the reasoning. Thanks for thinking things through.\n\n>\n> -- >8 --\n> Subject: [PATCH] shortlog: don't warn on empty author\n>\n> Git tries to avoid creating a commit with an empty author\n> name or email. However, commits created by older, less\n> strict versions of git may still be in the history. There's\n> not much point in issuing a warning to stderr for an empty\n> author. The user can't do anything about it now, and we are\n> better off to simply include it in the shortlog output as an\n> empty name/email, and let the caller process it however they\n> see fit.\n>\n> Older versions of shortlog differentiated between \"author\n> header not present\" (which complained) and \"author\n> name/email are blank\" (which included the empty ident in the\n> output). But since switching to format_commit_message, we\n> complain to stderr about either case (linux.git has a blank\n> author deep in its history which triggers this).\n>\n> We could try to restore the older behavior (complaining only\n> about the missing header), but in retrospect, there's not\n> much point in differentiating these cases. A missing\n> author header is bogus, but as for the \"blank\" case, the\n> only useful behavior is to add it to the \"empty name\"\n> collection.\n>\n> Signed-off-by: Jeff King \n> ---\n> builtin/shortlog.c | 8 --------\n> t/t4201-shortlog.sh | 16 ----------------\n> 2 files changed, 24 deletions(-)\n>\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index adbf1fd..e32be39 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -149,13 +149,6 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n> \tctx.output_encoding = get_log_output_encoding();\n> \n> \tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n> -\t/* we can detect a total failure only by seeing \" <>\" in the output */\n> -\tif (author.len <= 3) {\n> -\t\twarning(_(\"Missing author: %s\"),\n> -\t\t oid_to_hex(&commit->object.oid));\n> -\t\tgoto out;\n> -\t}\n> -\n> \tif (!log->summary) {\n> \t\tif (log->user_format)\n> \t\t\tpretty_print_commit(&ctx, commit, &oneline);\n> @@ -165,7 +158,6 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n> \n> \tinsert_one_record(log, author.buf, oneline.len ? oneline.buf : \"\");\n> \n> -out:\n> \tstrbuf_release(&author);\n> \tstrbuf_release(&oneline);\n> }\n> diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\n> index 82b2314..f5e6367 100755\n> --- a/t/t4201-shortlog.sh\n> +++ b/t/t4201-shortlog.sh\n> @@ -178,22 +178,6 @@ test_expect_success !MINGW 'shortlog encoding' '\n> \tgit shortlog HEAD~2.. > out &&\n> test_cmp expect out'\n> \n> -test_expect_success 'shortlog ignores commits with missing authors' '\n> -\tgit commit --allow-empty -m normal &&\n> -\tgit commit --allow-empty -m soon-to-be-broken &&\n> -\tgit cat-file commit HEAD >commit.tmp &&\n> -\tsed \"/^author/d\" commit.tmp >broken.tmp &&\n> -\tcommit=$(git hash-object -w -t commit --stdin -\tgit update-ref HEAD $commit &&\n> -\tcat >expect <<-\\EOF &&\n> -\tA U Thor (1):\n> -\t normal\n> -\n> -\tEOF\n> -\tgit shortlog HEAD~2.. >actual &&\n> -\ttest_cmp expect actual\n> -'\n> -\n> test_expect_success 'shortlog with revision pseudo options' '\n> \tgit shortlog --all &&\n> \tgit shortlog --branches &&\n"}]}