{"thread":{"id":"54293","subject":"[PATCH 0/8] parsing trailers with shortlog","startedAt":"2020-09-25T07:01:22Z","lastAt":"2021-02-04T06:45:19Z","messageCount":44,"participants":["Jeff King","Derrick Stolee","Junio C Hamano","Eric Sunshine","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"406306","messageId":"20200925070120.GA3669667@coredump.intra.peff.net","threadId":"54293","inReplyTo":null,"subject":"[PATCH 0/8] parsing trailers with shortlog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:01:20Z","receivedAt":"2020-09-25T07:01:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Somebody mentioned at the inclusion summit that it would be nice for\nmentoring/pairing relationships if we could more easily give credit to\nmultiple authors of a commit. When people ask about adding multiple\n\"author\" headers, we usually recommend that they use a \"co-authored-by\"\ntrailer. But you can't convince shortlog to count it for anything. :)\n\nSo this series adds support for counting trailers to shortlog. You can\ndo a number of fun things with it, like:\n\n # credit reviewers\n git shortlog -ns --group=trailer:reviewed-by\n\n # credit authors and co-authors equally\n git shortlog -ns --group=author \\\n --group=trailer:co-authored-by\n\n # see who helps whom\n git shortlog --format=\"...helped %an on %as\" \\\n --group=trailer:helped-by\n\nIf some of this looks familiar, it's because I sent the early patches\nyears ago. But I won't even bother linking to it; I cleaned up quite a\nfew rough edges since then, so it's not really worth looking at.\n\n [1/8]: shortlog: change \"author\" variables to \"ident\"\n [2/8]: shortlog: refactor committer/author grouping\n [3/8]: trailer: add interface for iterating over commit trailers\n [4/8]: shortlog: match commit trailers with --group\n [5/8]: shortlog: de-duplicate trailer values\n [6/8]: shortlog: rename parse_stdin_ident()\n [7/8]: shortlog: parse trailer idents\n [8/8]: shortlog: allow multiple groups to be specified\n\n Documentation/git-shortlog.txt | 29 +++++\n builtin/log.c | 1 +\n builtin/shortlog.c | 218 +++++++++++++++++++++++++++++----\n shortlog.h | 8 +-\n t/t4201-shortlog.sh | 141 +++++++++++++++++++++\n trailer.c | 36 ++++++\n trailer.h | 44 ++++++-\n 7 files changed, 447 insertions(+), 30 deletions(-)\n\n-Peff\n"},{"id":"406307","messageId":"20200925070150.GA62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 1/8] shortlog: change \"author\" variables to \"ident\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:01:50Z","receivedAt":"2020-09-25T07:01:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already match \"committer\", and we're about to start\nmatching more things. Let's use a more neutral variable to\navoid confusion.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex c856c58bb5..edcf2e0d54 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -49,12 +49,12 @@ static int compare_by_list(const void *a1, const void *a2)\n }\n \n static void insert_one_record(struct shortlog *log,\n-\t\t\t const char *author,\n+\t\t\t const char *ident,\n \t\t\t const char *oneline)\n {\n \tstruct string_list_item *item;\n \n-\titem = string_list_insert(&log->list, author);\n+\titem = string_list_insert(&log->list, ident);\n \n \tif (log->summary)\n \t\titem->util = (void *)(UTIL_TO_INT(item) + 1);\n@@ -97,8 +97,8 @@ static void insert_one_record(struct shortlog *log,\n \t}\n }\n \n-static int parse_stdin_author(struct shortlog *log,\n-\t\t\t struct strbuf *out, const char *in)\n+static int parse_stdin_ident(struct shortlog *log,\n+\t\t\t struct strbuf *out, const char *in)\n {\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n@@ -122,18 +122,18 @@ static int parse_stdin_author(struct shortlog *log,\n \n static void read_from_stdin(struct shortlog *log)\n {\n-\tstruct strbuf author = STRBUF_INIT;\n-\tstruct strbuf mapped_author = STRBUF_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n+\tstruct strbuf mapped_ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstatic const char *author_match[2] = { \"Author: \", \"author \" };\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n \tmatch = log->committer ? committer_match : author_match;\n-\twhile (strbuf_getline_lf(&author, stdin) != EOF) {\n+\twhile (strbuf_getline_lf(&ident, stdin) != EOF) {\n \t\tconst char *v;\n-\t\tif (!skip_prefix(author.buf, match[0], &v) &&\n-\t\t !skip_prefix(author.buf, match[1], &v))\n+\t\tif (!skip_prefix(ident.buf, match[0], &v) &&\n+\t\t !skip_prefix(ident.buf, match[1], &v))\n \t\t\tcontinue;\n \t\twhile (strbuf_getline_lf(&oneline, stdin) != EOF &&\n \t\t oneline.len)\n@@ -142,20 +142,20 @@ static void read_from_stdin(struct shortlog *log)\n \t\t !oneline.len)\n \t\t\t; /* discard blanks */\n \n-\t\tstrbuf_reset(&mapped_author);\n-\t\tif (parse_stdin_author(log, &mapped_author, v) < 0)\n+\t\tstrbuf_reset(&mapped_ident);\n+\t\tif (parse_stdin_ident(log, &mapped_ident, v) < 0)\n \t\t\tcontinue;\n \n-\t\tinsert_one_record(log, mapped_author.buf, oneline.buf);\n+\t\tinsert_one_record(log, mapped_ident.buf, oneline.buf);\n \t}\n-\tstrbuf_release(&author);\n-\tstrbuf_release(&mapped_author);\n+\tstrbuf_release(&ident);\n+\tstrbuf_release(&mapped_ident);\n \tstrbuf_release(&oneline);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n-\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tconst char *fmt;\n@@ -170,17 +170,17 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t(log->email ? \"%cN <%cE>\" : \"%cN\") :\n \t\t(log->email ? \"%aN <%aE>\" : \"%aN\");\n \n-\tformat_commit_message(commit, fmt, &author, &ctx);\n+\tformat_commit_message(commit, fmt, &ident, &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+\tinsert_one_record(log, ident.buf, oneline.len ? oneline.buf : \"\");\n \n-\tstrbuf_release(&author);\n+\tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n }\n \n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406308","messageId":"20200925070211.GB62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:02:11Z","receivedAt":"2020-09-25T07:02:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In preparation for adding more grouping types, let's\nrefactor the committer/author grouping code. In particular:\n\n - the master option is now \"--group\", to make it clear\n that the various group types are mutually exclusive. The\n \"--committer\" option is an alias for \"--group=committer\".\n\n - we keep an enum rather than a binary flag, to prepare\n for more values\n\n - we prefer switch statements to ternary assignment, since\n other group types will need more custom code\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 9 ++++++\n builtin/shortlog.c | 59 +++++++++++++++++++++++++++-------\n shortlog.h | 6 +++-\n t/t4201-shortlog.sh | 5 +++\n 4 files changed, 67 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex a72ea7f7ba..41b7679aa1 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -47,9 +47,18 @@ OPTIONS\n \n \tEach pretty-printed commit will be rewrapped before it is shown.\n \n+--group=::\n+\tBy default, `shortlog` collects and collates author identities;\n+\tusing `--group` will collect other fields.\n+\t`` is one of:\n++\n+ - `author`, commits are grouped by author (this is the default)\n+ - `committer`, commits are grouped by committer (the same as `-c`)\n+\n -c::\n --committer::\n \tCollect and show committer identities instead of authors.\n+\tThis is an alias for `--group=committer`.\n \n -w[[,[,]]]::\n \tLinewrap the output by wrapping each line at `width`. The first\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex edcf2e0d54..880ce19304 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -129,7 +129,17 @@ static void read_from_stdin(struct shortlog *log)\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n-\tmatch = log->committer ? committer_match : author_match;\n+\tswitch (log->group) {\n+\tcase SHORTLOG_GROUP_AUTHOR:\n+\t\tmatch = author_match;\n+\t\tbreak;\n+\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tmatch = committer_match;\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"unhandled shortlog group\");\n+\t}\n+\n \twhile (strbuf_getline_lf(&ident, stdin) != EOF) {\n \t\tconst char *v;\n \t\tif (!skip_prefix(ident.buf, match[0], &v) &&\n@@ -158,27 +168,36 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n-\tconst char *fmt;\n+\tconst char *oneline_str;\n \n \tctx.fmt = CMIT_FMT_USERFORMAT;\n \tctx.abbrev = log->abbrev;\n \tctx.print_email_subject = 1;\n \tctx.date_mode.type = DATE_NORMAL;\n \tctx.output_encoding = get_log_output_encoding();\n \n-\tfmt = log->committer ?\n-\t\t(log->email ? \"%cN <%cE>\" : \"%cN\") :\n-\t\t(log->email ? \"%aN <%aE>\" : \"%aN\");\n-\n-\tformat_commit_message(commit, fmt, &ident, &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, ident.buf, oneline.len ? oneline.buf : \"\");\n+\toneline_str = oneline.len ? oneline.buf : \"\";\n+\n+\tswitch (log->group) {\n+\tcase SHORTLOG_GROUP_AUTHOR:\n+\t\tformat_commit_message(commit,\n+\t\t\t\t log->email ? \"%aN <%aE>\" : \"%aN\",\n+\t\t\t\t &ident, &ctx);\n+\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t\tbreak;\n+\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tformat_commit_message(commit,\n+\t\t\t\t log->email ? \"%cN <%cE>\" : \"%cN\",\n+\t\t\t\t &ident, &ctx);\n+\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t\tbreak;\n+\t}\n \n \tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n@@ -241,6 +260,21 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static int parse_group_option(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct shortlog *log = opt->value;\n+\n+\tif (unset || !strcasecmp(arg, \"author\"))\n+\t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n+\telse if (!strcasecmp(arg, \"committer\"))\n+\t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n+\telse\n+\t\treturn error(_(\"unknown group type: %s\"), arg);\n+\n+\treturn 0;\n+}\n+\n+\n void shortlog_init(struct shortlog *log)\n {\n \tmemset(log, 0, sizeof(*log));\n@@ -260,8 +294,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n-\t\tOPT_BOOL('c', \"committer\", &log.committer,\n-\t\t\t N_(\"Group by committer rather than author\")),\n+\t\tOPT_SET_INT('c', \"committer\", &log.group,\n+\t\t\t N_(\"Group by committer rather than author\"),\n+\t\t\t SHORTLOG_GROUP_COMMITTER),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\n@@ -271,6 +306,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK_F('w', NULL, &log, N_(\"[,[,]]\"),\n \t\t\tN_(\"Linewrap output\"), PARSE_OPT_OPTARG,\n \t\t\t&parse_wrap_args),\n+\t\tOPT_CALLBACK(0, \"group\", &log, N_(\"field\"),\n+\t\t\tN_(\"Group by field\"), parse_group_option),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/shortlog.h b/shortlog.h\nindex 2fa61c4294..9deeeb3ffb 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -15,7 +15,11 @@ struct shortlog {\n \tint in2;\n \tint user_format;\n \tint abbrev;\n-\tint committer;\n+\n+\tenum {\n+\t\tSHORTLOG_GROUP_AUTHOR = 0,\n+\t\tSHORTLOG_GROUP_COMMITTER\n+\t} group;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex d3a7ce6bbb..65e4468746 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -215,4 +215,9 @@ test_expect_success 'shortlog --committer (external)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--group=committer is the same as --committer' '\n+\tgit shortlog -ns --group=committer HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406309","messageId":"20200925070249.GC62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 3/8] trailer: add interface for iterating over commit trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:02:49Z","receivedAt":"2020-09-25T07:02:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The trailer code knows how to parse out the trailers and re-format them,\nbut there's no easy way to iterate over the trailers (you can use\ntrailer_info, but you have to then do a bunch of extra parsing).\n\nLet's add an iteration interface that makes this easy to do.\n\nSigned-off-by: Jeff King \n---\n trailer.c | 36 ++++++++++++++++++++++++++++++++++++\n trailer.h | 44 ++++++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 78 insertions(+), 2 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex 68dabc2556..3f7391d793 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1183,3 +1183,39 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg,\n \tformat_trailer_info(out, &info, opts);\n \ttrailer_info_release(&info);\n }\n+\n+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)\n+{\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tstrbuf_init(&iter->key, 0);\n+\tstrbuf_init(&iter->val, 0);\n+\topts.no_divider = 1;\n+\ttrailer_info_get(&iter->info, msg, &opts);\n+\titer->cur = 0;\n+}\n+\n+int trailer_iterator_advance(struct trailer_iterator *iter)\n+{\n+\twhile (iter->cur < iter->info.trailer_nr) {\n+\t\tchar *trailer = iter->info.trailers[iter->cur++];\n+\t\tint separator_pos = find_separator(trailer, separators);\n+\n+\t\tif (separator_pos < 1)\n+\t\t\tcontinue; /* not a real trailer */\n+\n+\t\tstrbuf_reset(&iter->key);\n+\t\tstrbuf_reset(&iter->val);\n+\t\tparse_trailer(&iter->key, &iter->val, NULL,\n+\t\t\t trailer, separator_pos);\n+\t\tunfold_value(&iter->val);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+void trailer_iterator_release(struct trailer_iterator *iter)\n+{\n+\ttrailer_info_release(&iter->info);\n+\tstrbuf_release(&iter->val);\n+\tstrbuf_release(&iter->key);\n+}\ndiff --git a/trailer.h b/trailer.h\nindex 203acf4ee1..af9949363a 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -2,8 +2,7 @@\n #define TRAILER_H\n \n #include \"list.h\"\n-\n-struct strbuf;\n+#include \"strbuf.h\"\n \n enum trailer_where {\n \tWHERE_DEFAULT,\n@@ -103,4 +102,45 @@ void trailer_info_release(struct trailer_info *info);\n void format_trailers_from_commit(struct strbuf *out, const char *msg,\n \t\t\t\t const struct process_trailer_options *opts);\n \n+/*\n+ * An interface for iterating over the trailers found in a particular commit\n+ * message. Use like:\n+ *\n+ * struct trailer_iterator iter;\n+ * trailer_iterator_init(&iter, msg);\n+ * while (trailer_iterator_advance(&iter))\n+ * ... do something with iter.key and iter.val ...\n+ * trailer_iterator_release(&iter);\n+ */\n+struct trailer_iterator {\n+\tstruct strbuf key;\n+\tstruct strbuf val;\n+\n+\t/* private */\n+\tstruct trailer_info info;\n+\tsize_t cur;\n+};\n+\n+/*\n+ * Initialize \"iter\" in preparation for walking over the trailers in the commit\n+ * message \"msg\". The \"msg\" pointer must remain valid until the iterator is\n+ * released.\n+ *\n+ * After initializing, we are not yet pointing\n+ */\n+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg);\n+\n+/*\n+ * Advance to the next trailer of the iterator. Returns 0 if there is no such\n+ * trailer, and 1 otherwise. The key and value of the trailer can be\n+ * fetched from the iter->key and iter->value fields (which are valid\n+ * only until the next advance).\n+ */\n+int trailer_iterator_advance(struct trailer_iterator *iter);\n+\n+/*\n+ * Release all resources associated with the trailer iteration.\n+ */\n+void trailer_iterator_release(struct trailer_iterator *iter);\n+\n #endif /* TRAILER_H */\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406310","messageId":"20200925070343.GD62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 4/8] shortlog: match commit trailers with --group","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:03:43Z","receivedAt":"2020-09-25T07:03:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If a project uses commit trailers, this patch lets you use\nshortlog to see who is performing each action. For example,\nrunning:\n\n git shortlog -ns --group=trailer:reviewed-by\n\nin git.git shows who has reviewed. You can even use a custom\nformat to see things like who has helped whom:\n\n git shortlog --format=\"...helped %an (%ad)\" \\\n --group=trailer:helped-by\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 13 ++++++++++\n builtin/shortlog.c | 44 +++++++++++++++++++++++++++++++++-\n shortlog.h | 4 +++-\n t/t4201-shortlog.sh | 14 +++++++++++\n 4 files changed, 73 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 41b7679aa1..40752bc573 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -54,6 +54,19 @@ OPTIONS\n +\n - `author`, commits are grouped by author (this is the default)\n - `committer`, commits are grouped by committer (the same as `-c`)\n+ - `trailer:`, the `` is interpreted as a case-insensitive\n+ commit message trailer (see linkgit:git-interpret-trailers[1]). For\n+ example, if your project uses `Reviewed-by` trailers, you might want\n+ to see who has been reviewing with\n+ `git shortlog -ns --group=trailer:reviewed-by`.\n++\n+Note that commits that do not include the trailer will not be counted.\n+Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n+be counted more than once.\n++\n+The contents of each trailer value are taken literally and completely.\n+No mailmap is applied, and the `-e` option has no effect (if the trailer\n+contains a username and email, they are both always shown).\n \n -c::\n --committer::\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 880ce19304..e1d9ee909f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -9,6 +9,7 @@\n #include \"mailmap.h\"\n #include \"shortlog.h\"\n #include \"parse-options.h\"\n+#include \"trailer.h\"\n \n static char const * const shortlog_usage[] = {\n \tN_(\"git shortlog [] [] [[--] ...]\"),\n@@ -136,6 +137,8 @@ static void read_from_stdin(struct shortlog *log)\n \tcase SHORTLOG_GROUP_COMMITTER:\n \t\tmatch = committer_match;\n \t\tbreak;\n+\tcase SHORTLOG_GROUP_TRAILER:\n+\t\tdie(_(\"using --group=trailer with stdin is not supported\"));\n \tdefault:\n \t\tBUG(\"unhandled shortlog group\");\n \t}\n@@ -163,6 +166,37 @@ static void read_from_stdin(struct shortlog *log)\n \tstrbuf_release(&oneline);\n }\n \n+static void insert_records_from_trailers(struct shortlog *log,\n+\t\t\t\t\t struct commit *commit,\n+\t\t\t\t\t struct pretty_print_context *ctx,\n+\t\t\t\t\t const char *oneline)\n+{\n+\tstruct trailer_iterator iter;\n+\tconst char *commit_buffer, *body;\n+\n+\t/*\n+\t * Using format_commit_message(\"%B\") would be simpler here, but\n+\t * this saves us copying the message.\n+\t */\n+\tcommit_buffer = logmsg_reencode(commit, NULL, ctx->output_encoding);\n+\tbody = strstr(commit_buffer, \"\\n\\n\");\n+\tif (!body)\n+\t\treturn;\n+\n+\ttrailer_iterator_init(&iter, body);\n+\twhile (trailer_iterator_advance(&iter)) {\n+\t\tconst char *value = iter.val.buf;\n+\n+\t\tif (strcasecmp(iter.key.buf, log->trailer))\n+\t\t\tcontinue;\n+\n+\t\tinsert_one_record(log, value, oneline);\n+\t}\n+\ttrailer_iterator_release(&iter);\n+\n+\tunuse_commit_buffer(commit, commit_buffer);\n+}\n+\n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n \tstruct strbuf ident = STRBUF_INIT;\n@@ -197,6 +231,9 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t\t\t &ident, &ctx);\n \t\tinsert_one_record(log, ident.buf, oneline_str);\n \t\tbreak;\n+\tcase SHORTLOG_GROUP_TRAILER:\n+\t\tinsert_records_from_trailers(log, commit, &ctx, oneline_str);\n+\t\tbreak;\n \t}\n \n \tstrbuf_release(&ident);\n@@ -263,12 +300,17 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)\n static int parse_group_option(const struct option *opt, const char *arg, int unset)\n {\n \tstruct shortlog *log = opt->value;\n+\tconst char *field;\n \n \tif (unset || !strcasecmp(arg, \"author\"))\n \t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n \telse if (!strcasecmp(arg, \"committer\"))\n \t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n-\telse\n+\telse if (skip_prefix(arg, \"trailer:\", &field)) {\n+\t\tlog->group = SHORTLOG_GROUP_TRAILER;\n+\t\tfree(log->trailer);\n+\t\tlog->trailer = xstrdup(field);\n+\t} else\n \t\treturn error(_(\"unknown group type: %s\"), arg);\n \n \treturn 0;\ndiff --git a/shortlog.h b/shortlog.h\nindex 9deeeb3ffb..54ce55e9e9 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -18,8 +18,10 @@ struct shortlog {\n \n \tenum {\n \t\tSHORTLOG_GROUP_AUTHOR = 0,\n-\t\tSHORTLOG_GROUP_COMMITTER\n+\t\tSHORTLOG_GROUP_COMMITTER,\n+\t\tSHORTLOG_GROUP_TRAILER\n \t} group;\n+\tchar *trailer;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 65e4468746..e97d891a71 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -220,4 +220,18 @@ test_expect_success '--group=committer is the same as --committer' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog --group=trailer:signed-off-by' '\n+\tgit commit --allow-empty -m foo -s &&\n+\tGIT_COMMITTER_NAME=\"SOB One\" \\\n+\tGIT_COMMITTER_EMAIL=sob@example.com \\\n+\t\tgit commit --allow-empty -m foo -s &&\n+\tgit commit --allow-empty --amend --no-edit -s &&\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter \n+\t 1\tSOB One \n+\tEOF\n+\tgit shortlog -ns --group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406311","messageId":"20200925070509.GE62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 5/8] shortlog: de-duplicate trailer values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:05:09Z","receivedAt":"2020-09-25T07:05:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current documentation is vague about what happens with\n--group=trailer:signed-off-by when we see a commit with:\n\n Signed-off-by: One\n Signed-off-by: Two\n Signed-off-by: One\n\nWe clearly should credit both \"One\" and \"Two\", but should \"One\" get\ncredited twice? The current code does so, but mostly because that was\nthe easiest thing to do. It's probably more useful to count each commit\nat most once. This will become especially important when we allow\nvalues from multiple sources in a future patch.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 3 +-\n builtin/shortlog.c | 58 ++++++++++++++++++++++++++++++++++\n t/t4201-shortlog.sh | 28 ++++++++++++++++\n 3 files changed, 88 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 40752bc573..29a9b22d3b 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -62,7 +62,8 @@ OPTIONS\n +\n Note that commits that do not include the trailer will not be counted.\n Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n-be counted more than once.\n+be counted more than once (but only once per unique trailer value in\n+that commit).\n +\n The contents of each trailer value are taken literally and completely.\n No mailmap is applied, and the `-e` option has no effect (if the trailer\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e1d9ee909f..d2d8103dd3 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -166,13 +166,68 @@ static void read_from_stdin(struct shortlog *log)\n \tstrbuf_release(&oneline);\n }\n \n+struct strset_item {\n+\tstruct hashmap_entry ent;\n+\tchar value[FLEX_ARRAY];\n+};\n+\n+struct strset {\n+\tstruct hashmap map;\n+};\n+\n+#define STRSET_INIT { { NULL } }\n+\n+static int strset_item_hashcmp(const void *hash_data,\n+\t\t\t const struct hashmap_entry *entry,\n+\t\t\t const struct hashmap_entry *entry_or_key,\n+\t\t\t const void *keydata)\n+{\n+\tconst struct strset_item *a, *b;\n+\n+\ta = container_of(entry, const struct strset_item, ent);\n+\tif (keydata)\n+\t\treturn strcmp(a->value, keydata);\n+\n+\tb = container_of(entry_or_key, const struct strset_item, ent);\n+\treturn strcmp(a->value, b->value);\n+}\n+\n+/*\n+ * Adds \"str\" to the set if it was not already present; returns true if it was\n+ * already there.\n+ */\n+static int strset_check_and_add(struct strset *ss, const char *str)\n+{\n+\tunsigned int hash = strhash(str);\n+\tstruct strset_item *item;\n+\n+\tif (!ss->map.table)\n+\t\thashmap_init(&ss->map, strset_item_hashcmp, NULL, 0);\n+\n+\tif (hashmap_get_from_hash(&ss->map, hash, str))\n+\t\treturn 1;\n+\n+\tFLEX_ALLOC_STR(item, value, str);\n+\thashmap_entry_init(&item->ent, hash);\n+\thashmap_add(&ss->map, &item->ent);\n+\treturn 0;\n+}\n+\n+static void strset_clear(struct strset *ss)\n+{\n+\tif (!ss->map.table)\n+\t\treturn;\n+\thashmap_free_entries(&ss->map, struct strset_item, ent);\n+}\n+\n static void insert_records_from_trailers(struct shortlog *log,\n \t\t\t\t\t struct commit *commit,\n \t\t\t\t\t struct pretty_print_context *ctx,\n \t\t\t\t\t const char *oneline)\n {\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n+\tstruct strset dups = STRSET_INIT;\n \n \t/*\n \t * Using format_commit_message(\"%B\") would be simpler here, but\n@@ -190,10 +245,13 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\tif (strcasecmp(iter.key.buf, log->trailer))\n \t\t\tcontinue;\n \n+\t\tif (strset_check_and_add(&dups, value))\n+\t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n+\tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\n \ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex e97d891a71..83dbbc44e8 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -234,4 +234,32 @@ test_expect_success 'shortlog --group=trailer:signed-off-by' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject one\n+\n+\tthis message has two distinct values, plus a repeat\n+\n+\tRepeated-trailer: Foo\n+\tRepeated-trailer: Bar\n+\tRepeated-trailer: Foo\n+\tEOF\n+\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject two\n+\n+\tsimilar to the previous, but without the second distinct value\n+\n+\tRepeated-trailer: Foo\n+\tRepeated-trailer: Foo\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\t 2\tFoo\n+\t 1\tBar\n+\tEOF\n+\tgit shortlog -ns --group=trailer:repeated-trailer -2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406312","messageId":"20200925070518.GF62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 6/8] shortlog: rename parse_stdin_ident()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:05:18Z","receivedAt":"2020-09-25T07:05:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function is actually useful for parsing any identity, whether from\nstdin or not. We'll need it for handling trailers.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex d2d8103dd3..e6f4faec7c 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -98,8 +98,8 @@ static void insert_one_record(struct shortlog *log,\n \t}\n }\n \n-static int parse_stdin_ident(struct shortlog *log,\n-\t\t\t struct strbuf *out, const char *in)\n+static int parse_ident(struct shortlog *log,\n+\t\t struct strbuf *out, const char *in)\n {\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n@@ -156,7 +156,7 @@ static void read_from_stdin(struct shortlog *log)\n \t\t\t; /* discard blanks */\n \n \t\tstrbuf_reset(&mapped_ident);\n-\t\tif (parse_stdin_ident(log, &mapped_ident, v) < 0)\n+\t\tif (parse_ident(log, &mapped_ident, v) < 0)\n \t\t\tcontinue;\n \n \t\tinsert_one_record(log, mapped_ident.buf, oneline.buf);\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406313","messageId":"20200925070531.GG62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 7/8] shortlog: parse trailer idents","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:05:31Z","receivedAt":"2020-09-25T07:05:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Trailers don't necessarily contain name/email identity values, so\nshortlog has so far treated them as opaque strings. However, since many\ntrailers do contain identities, it's useful to treat them as such when\nthey can be parsed. That lets \"-e\" work as usual, as well as mailmap.\n\nWhen they can't be parsed, we'll continue with the old behavior of\ntreating them as a single string (there's no new test for that here,\nsince the existing tests cover a trailer like this).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 7 ++++---\n builtin/shortlog.c | 6 ++++++\n t/t4201-shortlog.sh | 20 ++++++++++++++++++++\n 3 files changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 29a9b22d3b..5f8f918cad 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -65,9 +65,10 @@ Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n be counted more than once (but only once per unique trailer value in\n that commit).\n +\n-The contents of each trailer value are taken literally and completely.\n-No mailmap is applied, and the `-e` option has no effect (if the trailer\n-contains a username and email, they are both always shown).\n+Shortlog will attempt to parse each trailer value as a `name `\n+identity. If successful, the mailmap is applied and the email is omitted\n+unless the `--email` option is specified. If the value cannot be parsed\n+as an identity, it will be taken literally and completely.\n \n -c::\n --committer::\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e6f4faec7c..28133aec68 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -228,6 +228,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n \tstruct strset dups = STRSET_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n \n \t/*\n \t * Using format_commit_message(\"%B\") would be simpler here, but\n@@ -245,12 +246,17 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\tif (strcasecmp(iter.key.buf, log->trailer))\n \t\t\tcontinue;\n \n+\t\tstrbuf_reset(&ident);\n+\t\tif (!parse_ident(log, &ident, value))\n+\t\t\tvalue = ident.buf;\n+\n \t\tif (strset_check_and_add(&dups, value))\n \t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n+\tstrbuf_release(&ident);\n \tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 83dbbc44e8..a62ee9ed55 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -230,10 +230,30 @@ test_expect_success 'shortlog --group=trailer:signed-off-by' '\n \t 2\tC O Mitter \n \t 1\tSOB One \n \tEOF\n+\tgit shortlog -nse --group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'trailer idents are split' '\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter\n+\t 1\tSOB One\n+\tEOF\n \tgit shortlog -ns --group=trailer:signed-off-by HEAD >actual &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'trailer idents are mailmapped' '\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter\n+\t 1\tAnother Name\n+\tEOF\n+\techo \"Another Name \" >mail.map &&\n+\tgit -c mailmap.file=mail.map shortlog -ns \\\n+\t\t--group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n \tgit commit --allow-empty -F - <<-\\EOF &&\n \tsubject one\n-- \n2.28.0.1085.g44a0350633\n\n"},{"id":"406314","messageId":"20200925070550.GH62741@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH 8/8] shortlog: allow multiple groups to be specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T07:05:50Z","receivedAt":"2020-09-25T07:05:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that shortlog supports reading from trailers, it can be useful to\ncombine counts from multiple trailers, or between trailers and authors.\nThis can be done manually by post-processing the output from multiple\nruns, but it's non-trivial to make sure that each name/commit pair is\ncounted only once.\n\nThis patch teaches shortlog to accept multiple --group options on the\ncommand line, and pull data from all of them. That makes it possible to\nrun:\n\n git shortlog -ns --group=author --group=trailer:co-authored-by\n\nto get a shortlog that counts authors and co-authors equally.\n\nThe implementation is mostly straightforward. The \"group\" enum becomes a\nbitfield, and the trailer key becomes a list. I didn't bother\nimplementing the multi-group semantics for reading from stdin. It would\nbe possible to do, but the existing matching code makes it awkward, and\nI doubt anybody cares.\n\nThe duplicate suppression we used for trailers now covers authors and\ncommitters as well (though in non-trailer single-group mode we can skip\nthe hash insertion and lookup, since we only see one value per commit).\n\nThere is one subtlety: we now care about the case when no group bit is\nset (in which case we default to showing the author). The caller in\nbuiltin/log.c needs to be adapted to ask explicitly for authors, rather\nthan relying on shortlog_init(). It would be possible with some\ngymnastics to make this keep working as-is, but it's not worth it for a\nsingle caller.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 5 +++\n builtin/log.c | 1 +\n builtin/shortlog.c | 69 ++++++++++++++++++++-----------\n shortlog.h | 10 ++---\n t/t4201-shortlog.sh | 74 ++++++++++++++++++++++++++++++++++\n 5 files changed, 130 insertions(+), 29 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 5f8f918cad..f31770dab5 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -69,6 +69,11 @@ Shortlog will attempt to parse each trailer value as a `name `\n identity. If successful, the mailmap is applied and the email is omitted\n unless the `--email` option is specified. If the value cannot be parsed\n as an identity, it will be taken literally and completely.\n++\n+If `--group` is specified multiple times, commits are counted under each\n+value (but again, only once per unique value in that commit). For\n+example, `git shortlog --group=author --group=trailer:co-authored-by`\n+counts both authors and co-authors.\n \n -c::\n --committer::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b8824d898f..7f27e9eca1 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1195,6 +1195,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tlog.in1 = 2;\n \tlog.in2 = 4;\n \tlog.file = rev->diffopt.file;\n+\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n \tfor (i = 0; i < nr; i++)\n \t\tshortlog_add_commit(&log, list[i]);\n \ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 28133aec68..a36a8717b7 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -121,6 +121,11 @@ static int parse_ident(struct shortlog *log,\n \treturn 0;\n }\n \n+static inline int has_multi_bits(unsigned n)\n+{\n+\treturn (n & (n - 1)) != 0;\n+}\n+\n static void read_from_stdin(struct shortlog *log)\n {\n \tstruct strbuf ident = STRBUF_INIT;\n@@ -130,7 +135,10 @@ static void read_from_stdin(struct shortlog *log)\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n-\tswitch (log->group) {\n+\tif (has_multi_bits(log->groups))\n+\t\tdie(_(\"using multiple --group options with stdin is not supported\"));\n+\n+\tswitch (log->groups) {\n \tcase SHORTLOG_GROUP_AUTHOR:\n \t\tmatch = author_match;\n \t\tbreak;\n@@ -221,13 +229,13 @@ static void strset_clear(struct strset *ss)\n }\n \n static void insert_records_from_trailers(struct shortlog *log,\n+\t\t\t\t\t struct strset *dups,\n \t\t\t\t\t struct commit *commit,\n \t\t\t\t\t struct pretty_print_context *ctx,\n \t\t\t\t\t const char *oneline)\n {\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n-\tstruct strset dups = STRSET_INIT;\n \tstruct strbuf ident = STRBUF_INIT;\n \n \t/*\n@@ -243,28 +251,28 @@ static void insert_records_from_trailers(struct shortlog *log,\n \twhile (trailer_iterator_advance(&iter)) {\n \t\tconst char *value = iter.val.buf;\n \n-\t\tif (strcasecmp(iter.key.buf, log->trailer))\n+\t\tif (!string_list_has_string(&log->trailers, iter.key.buf))\n \t\t\tcontinue;\n \n \t\tstrbuf_reset(&ident);\n \t\tif (!parse_ident(log, &ident, value))\n \t\t\tvalue = ident.buf;\n \n-\t\tif (strset_check_and_add(&dups, value))\n+\t\tif (strset_check_and_add(dups, value))\n \t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n \tstrbuf_release(&ident);\n-\tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n \tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n+\tstruct strset dups = STRSET_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tconst char *oneline_str;\n \n@@ -282,24 +290,29 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t}\n \toneline_str = oneline.len ? oneline.buf : \"\";\n \n-\tswitch (log->group) {\n-\tcase SHORTLOG_GROUP_AUTHOR:\n+\tif (log->groups & SHORTLOG_GROUP_AUTHOR) {\n+\t\tstrbuf_reset(&ident);\n \t\tformat_commit_message(commit,\n \t\t\t\t log->email ? \"%aN <%aE>\" : \"%aN\",\n \t\t\t\t &ident, &ctx);\n-\t\tinsert_one_record(log, ident.buf, oneline_str);\n-\t\tbreak;\n-\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tif (!has_multi_bits(log->groups) ||\n+\t\t !strset_check_and_add(&dups, ident.buf))\n+\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t}\n+\tif (log->groups & SHORTLOG_GROUP_COMMITTER) {\n+\t\tstrbuf_reset(&ident);\n \t\tformat_commit_message(commit,\n \t\t\t\t log->email ? \"%cN <%cE>\" : \"%cN\",\n \t\t\t\t &ident, &ctx);\n-\t\tinsert_one_record(log, ident.buf, oneline_str);\n-\t\tbreak;\n-\tcase SHORTLOG_GROUP_TRAILER:\n-\t\tinsert_records_from_trailers(log, commit, &ctx, oneline_str);\n-\t\tbreak;\n+\t\tif (!has_multi_bits(log->groups) ||\n+\t\t !strset_check_and_add(&dups, ident.buf))\n+\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t}\n+\tif (log->groups & SHORTLOG_GROUP_TRAILER) {\n+\t\tinsert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);\n \t}\n \n+\tstrset_clear(&dups);\n \tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n }\n@@ -366,14 +379,16 @@ static int parse_group_option(const struct option *opt, const char *arg, int uns\n \tstruct shortlog *log = opt->value;\n \tconst char *field;\n \n-\tif (unset || !strcasecmp(arg, \"author\"))\n-\t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n+\tif (unset) {\n+\t\tlog->groups = 0;\n+\t\tstring_list_clear(&log->trailers, 0);\n+\t} else if (!strcasecmp(arg, \"author\"))\n+\t\tlog->groups |= SHORTLOG_GROUP_AUTHOR;\n \telse if (!strcasecmp(arg, \"committer\"))\n-\t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n+\t\tlog->groups |= SHORTLOG_GROUP_COMMITTER;\n \telse if (skip_prefix(arg, \"trailer:\", &field)) {\n-\t\tlog->group = SHORTLOG_GROUP_TRAILER;\n-\t\tfree(log->trailer);\n-\t\tlog->trailer = xstrdup(field);\n+\t\tlog->groups |= SHORTLOG_GROUP_TRAILER;\n+\t\tstring_list_append(&log->trailers, field);\n \t} else\n \t\treturn error(_(\"unknown group type: %s\"), arg);\n \n@@ -391,6 +406,8 @@ void shortlog_init(struct shortlog *log)\n \tlog->wrap = DEFAULT_WRAPLEN;\n \tlog->in1 = DEFAULT_INDENT1;\n \tlog->in2 = DEFAULT_INDENT2;\n+\tlog->trailers.strdup_strings = 1;\n+\tlog->trailers.cmp = strcasecmp;\n }\n \n int cmd_shortlog(int argc, const char **argv, const char *prefix)\n@@ -400,9 +417,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n-\t\tOPT_SET_INT('c', \"committer\", &log.group,\n-\t\t\t N_(\"Group by committer rather than author\"),\n-\t\t\t SHORTLOG_GROUP_COMMITTER),\n+\t\tOPT_BIT('c', \"committer\", &log.groups,\n+\t\t\tN_(\"Group by committer rather than author\"),\n+\t\t\tSHORTLOG_GROUP_COMMITTER),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\n@@ -454,6 +471,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tlog.abbrev = rev.abbrev;\n \tlog.file = rev.diffopt.file;\n \n+\tif (!log.groups)\n+\t\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n+\tstring_list_sort(&log.trailers);\n+\n \t/* assume HEAD if from a tty */\n \tif (!nongit && !rev.pending.nr && isatty(0))\n \t\tadd_head_to_pending(&rev);\ndiff --git a/shortlog.h b/shortlog.h\nindex 54ce55e9e9..030a0b8490 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -17,11 +17,11 @@ struct shortlog {\n \tint abbrev;\n \n \tenum {\n-\t\tSHORTLOG_GROUP_AUTHOR = 0,\n-\t\tSHORTLOG_GROUP_COMMITTER,\n-\t\tSHORTLOG_GROUP_TRAILER\n-\t} group;\n-\tchar *trailer;\n+\t\tSHORTLOG_GROUP_AUTHOR = (1 << 0),\n+\t\tSHORTLOG_GROUP_COMMITTER = (1 << 1),\n+\t\tSHORTLOG_GROUP_TRAILER = (1 << 2)\n+\t} groups;\n+\tstruct string_list trailers;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex a62ee9ed55..3d5c4a2086 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -282,4 +282,78 @@ test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog can match multiple groups' '\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject one\n+\n+\tthis has two trailers that are distinct from the author; it will count\n+\t3 times in the output\n+\n+\tSome-trailer: User A \n+\tAnother-trailer: User B \n+\tEOF\n+\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject two\n+\n+\tthis one has two trailers, one of which is a duplicate with the author;\n+\tit will only be counted once for them\n+\n+\tAnother-trailer: A U Thor \n+\tSome-trailer: User B \n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\t 2\tA U Thor\n+\t 2\tUser B\n+\t 1\tUser A\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=author \\\n+\t\t--group=trailer:some-trailer \\\n+\t\t--group=trailer:another-trailer \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'set up option selection tests' '\n+\tgit commit --allow-empty -F - <<-\\EOF\n+\tsubject\n+\n+\tbody\n+\n+\tTrailer-one: value-one\n+\tTrailer-two: value-two\n+\tEOF\n+'\n+\n+test_expect_success '--no-group resets group list to author' '\n+\tcat >expect <<-\\EOF &&\n+\t 1\tA U Thor\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=committer \\\n+\t\t--group=trailer:trailer-one \\\n+\t\t--no-group \\\n+\t\t-1 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-group resets trailer list' '\n+\tcat >expect <<-\\EOF &&\n+\t 1\tvalue-two\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=trailer:trailer-one \\\n+\t\t--no-group \\\n+\t\t--group=trailer:trailer-two \\\n+\t\t-1 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin with multiple groups reports error' '\n+\tgit log >log &&\n+\ttest_must_fail git shortlog --group=author --group=committer Somebody mentioned at the inclusion summit that it would be nice for\n> mentoring/pairing relationships if we could more easily give credit to\n> multiple authors of a commit. When people ask about adding multiple\n> \"author\" headers, we usually recommend that they use a \"co-authored-by\"\n> trailer. But you can't convince shortlog to count it for anything. :)\n\nThis discussion was worthwhile, and could even have measurable effects\non this community, depending on how Junio creates the list of\ncontributors for the release notes.\n\n> So this series adds support for counting trailers to shortlog. You can\n> do a number of fun things with it, like:\n> \n> # credit reviewers\n> git shortlog -ns --group=trailer:reviewed-by\n> \n> # credit authors and co-authors equally\n> git shortlog -ns --group=author \\\n> --group=trailer:co-authored-by\n> \n> # see who helps whom\n> git shortlog --format=\"...helped %an on %as\" \\\n> --group=trailer:helped-by\n\nI built Git with these patches and played around with these and\nother options (such as `--group=author --group=committer`). I\ncould not find any bugs or other ways to improve these patches.\n\nThanks,\n-Stolee\n"},{"id":"406350","messageId":"xmqqsgb5lrq2.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] parsing trailers with shortlog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T16:57:09Z","receivedAt":"2020-09-25T16:57:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Somebody mentioned at the inclusion summit that it would be nice for\n> mentoring/pairing relationships if we could more easily give credit to\n> multiple authors of a commit. When people ask about adding multiple\n> \"author\" headers, we usually recommend that they use a \"co-authored-by\"\n> trailer. But you can't convince shortlog to count it for anything. :)\n>\n> So this series adds support for counting trailers to shortlog. You can\n> do a number of fun things with it, like:\n>\n> # credit reviewers\n> git shortlog -ns --group=trailer:reviewed-by\n>\n> # credit authors and co-authors equally\n> git shortlog -ns --group=author \\\n> --group=trailer:co-authored-by\n>\n> # see who helps whom\n> git shortlog --format=\"...helped %an on %as\" \\\n> --group=trailer:helped-by\n>\n> If some of this looks familiar, it's because I sent the early patches\n> years ago. But I won't even bother linking to it; I cleaned up quite a\n> few rough edges since then, so it's not really worth looking at.\n\nFun stuff. I like the idea.\n\nThanks.\n\n\n>\n> [1/8]: shortlog: change \"author\" variables to \"ident\"\n> [2/8]: shortlog: refactor committer/author grouping\n> [3/8]: trailer: add interface for iterating over commit trailers\n> [4/8]: shortlog: match commit trailers with --group\n> [5/8]: shortlog: de-duplicate trailer values\n> [6/8]: shortlog: rename parse_stdin_ident()\n> [7/8]: shortlog: parse trailer idents\n> [8/8]: shortlog: allow multiple groups to be specified\n>\n> Documentation/git-shortlog.txt | 29 +++++\n> builtin/log.c | 1 +\n> builtin/shortlog.c | 218 +++++++++++++++++++++++++++++----\n> shortlog.h | 8 +-\n> t/t4201-shortlog.sh | 141 +++++++++++++++++++++\n> trailer.c | 36 ++++++\n> trailer.h | 44 ++++++-\n> 7 files changed, 447 insertions(+), 30 deletions(-)\n>\n> -Peff\n"},{"id":"406385","messageId":"CAPig+cTfYmNAP_65RDa_fZOyuQEH65HuLs-UYSPT0yJ=s6BftA@mail.gmail.com","threadId":"54293","inReplyTo":"20200925070211.GB62741@coredump.intra.peff.net","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T20:05:04Z","receivedAt":"2020-09-25T20:20:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 3:02 AM Jeff King wrote:\n> In preparation for adding more grouping types, let's\n> refactor the committer/author grouping code. In particular:\n>\n> - the master option is now \"--group\", to make it clear\n> that the various group types are mutually exclusive. The\n> \"--committer\" option is an alias for \"--group=committer\".\n>\n> - we keep an enum rather than a binary flag, to prepare\n> for more values\n>\n> - we prefer switch statements to ternary assignment, since\n> other group types will need more custom code\n>\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\n> @@ -47,9 +47,18 @@ OPTIONS\n> +--group=::\n> + By default, `shortlog` collects and collates author identities;\n> + using `--group` will collect other fields.\n> + `` is one of:\n> ++\n> + - `author`, commits are grouped by author (this is the default)\n> + - `committer`, commits are grouped by committer (the same as `-c`)\n\nI had trouble interpreting \"(this is the default)\". It made me think\nthat is optional:\n\n --group[=]\n\nbut that isn't the case at all. Instead, it means that if --group\nisn't specified, then grouping is done by `author` by default. It also\nrepeats what the general description of --group already says with\nregard to the default, thus it is redundant to say it again when\ndescribing the `author` type. Therefore, perhaps drop \"(this is the\ndefault)\" altogether?\n"},{"id":"406389","messageId":"CAPig+cS96Y8nMQgazZE-HP4Un2LpgfPF3-63vt8=WJf0c5EuRg@mail.gmail.com","threadId":"54293","inReplyTo":"20200925070550.GH62741@coredump.intra.peff.net","subject":"Re: [PATCH 8/8] shortlog: allow multiple groups to be specified","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T20:23:39Z","receivedAt":"2020-09-25T20:36:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 3:05 AM Jeff King wrote:\n> This patch teaches shortlog to accept multiple --group options on the\n> command line, and pull data from all of them. That makes it possible to\n> run:\n> [...]\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\n> @@ -69,6 +69,11 @@ Shortlog will attempt to parse each trailer value as a `name `\n> identity. If successful, the mailmap is applied and the email is omitted\n> unless the `--email` option is specified. If the value cannot be parsed\n> as an identity, it will be taken literally and completely.\n> ++\n> +If `--group` is specified multiple times, commits are counted under each\n> +value (but again, only once per unique value in that commit). For\n> +example, `git shortlog --group=author --group=trailer:co-authored-by`\n> +counts both authors and co-authors.\n\nIntuitively, I understand (or hope) that the first use of --group\noverrides what is otherwise collected by default (authors), however,\nwould there be value in stating this explicitly?\n\nAt this point, the documentation for --committer still says:\n\n --committer::\n Collect and show committer identities instead of authors.\n This is an alias for `--group=committer`.\n\nwhich stops feeling accurate now that --group can be specified more\nthan once. The \"instead of authors\" bit is particularly jarring. I\nwonder if the first sentence can simply be dropped.\n"},{"id":"406424","messageId":"CAN0heSoS837ZCLt8WLRBrthXgOErwz-vXW8Af9v2ZdCcCpWRiw@mail.gmail.com","threadId":"54293","inReplyTo":"20200925070211.GB62741@coredump.intra.peff.net","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-09-26T12:31:32Z","receivedAt":"2020-09-26T12:31:47Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Peff,\n\nSome random thoughts follow on this patch and a few others.\n\nOn Fri, 25 Sep 2020 at 09:04, Jeff King wrote:\n>\n> In preparation for adding more grouping types, let's\n> refactor the committer/author grouping code. In particular:\n>\n> - the master option is now \"--group\", to make it clear\n> that the various group types are mutually exclusive. The\n> \"--committer\" option is an alias for \"--group=committer\".\n\nI think this is more than a refactoring of the code. The user interface\nis also refactored (if that's even the right word). From the subject and\nthe first sentence above, I did not expect this first \"-\" item, nor that\nthe patch would touch Documentation/.\n\n> + enum {\n> + SHORTLOG_GROUP_AUTHOR = 0,\n> + SHORTLOG_GROUP_COMMITTER\n> + } group;\n\nYou could reduce the patch noise by adding a trailing comma, see\ncc0c42975a (\"CodingGuidelines: spell out post-C89 rules\", 2019-07-16).\n(I do realize that you later redefine all values anyway.)\n\nMartin\n"},{"id":"406425","messageId":"CAN0heSrhmf9-Ep+SWe7Wy-7c3M9-1NFBybSA=6aJ1vDYGTdciA@mail.gmail.com","threadId":"54293","inReplyTo":"20200925070249.GC62741@coredump.intra.peff.net","subject":"Re: [PATCH 3/8] trailer: add interface for iterating over commit trailers","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-09-26T12:39:32Z","receivedAt":"2020-09-26T12:39:46Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 25 Sep 2020 at 09:04, Jeff King wrote:\n>\n> The trailer code knows how to parse out the trailers and re-format them,\n> but there's no easy way to iterate over the trailers (you can use\n> trailer_info, but you have to then do a bunch of extra parsing).\n>\n> Let's add an iteration interface that makes this easy to do.\n\n> +void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)\n> +{\n> + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n> + strbuf_init(&iter->key, 0);\n> + strbuf_init(&iter->val, 0);\n> + opts.no_divider = 1;\n> + trailer_info_get(&iter->info, msg, &opts);\n> + iter->cur = 0;\n> +}\n\nOk, this does initialize everything...\n\n> +void trailer_iterator_release(struct trailer_iterator *iter)\n> +{\n> + trailer_info_release(&iter->info);\n> + strbuf_release(&iter->val);\n> + strbuf_release(&iter->key);\n> +}\n\n... and this side takes care of everything, too.\n\n> #define TRAILER_H\n>\n> #include \"list.h\"\n> -\n> -struct strbuf;\n> +#include \"strbuf.h\"\n\nSpotting and removing the forward declaration, ok.\n\n> +/*\n> + * An interface for iterating over the trailers found in a particular commit\n> + * message. Use like:\n> + *\n> + * struct trailer_iterator iter;\n> + * trailer_iterator_init(&iter, msg);\n> + * while (trailer_iterator_advance(&iter))\n> + * ... do something with iter.key and iter.val ...\n> + * trailer_iterator_release(&iter);\n> + */\n> +struct trailer_iterator {\n> + struct strbuf key;\n> + struct strbuf val;\n> +\n> + /* private */\n> + struct trailer_info info;\n> + size_t cur;\n> +};\n\nOk.\n\n> +/*\n> + * Initialize \"iter\" in preparation for walking over the trailers in the commit\n> + * message \"msg\". The \"msg\" pointer must remain valid until the iterator is\n> + * released.\n> + *\n> + * After initializing, we are not yet pointing\n> + */\n\nTruncated sentence. \"... not yet pointing at any trailer\"?\n\nMartin\n"},{"id":"406426","messageId":"CAN0heSp5ssE1_-0B4VMGcuOaW63EpWXRVp4E4gy_7zVZ1rWkew@mail.gmail.com","threadId":"54293","inReplyTo":"20200925070550.GH62741@coredump.intra.peff.net","subject":"Re: [PATCH 8/8] shortlog: allow multiple groups to be specified","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-09-26T12:48:44Z","receivedAt":"2020-09-26T12:48:59Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 25 Sep 2020 at 09:09, Jeff King wrote:\n> +static inline int has_multi_bits(unsigned n)\n> +{\n> + return (n & (n - 1)) != 0;\n> +}\n\nThere's a HAS_MULTI_BITS macro in git-compat-util.h. I like how you came\nup with the exact same name. It even makes me wonder if you are actively\navoiding it in favor of a re-implementation, but I can't see why.\n\n> enum {\n> - SHORTLOG_GROUP_AUTHOR = 0,\n> - SHORTLOG_GROUP_COMMITTER,\n> - SHORTLOG_GROUP_TRAILER\n> - } group;\n> + SHORTLOG_GROUP_AUTHOR = (1 << 0),\n> + SHORTLOG_GROUP_COMMITTER = (1 << 1),\n> + SHORTLOG_GROUP_TRAILER = (1 << 2)\n> + } groups;\n\nComing back to the comment on 2/8, adding a comma at the end wouldn't\nreduce patch noise here and now, but whenever someone touches this place\nthe next time.\n\nOf my comments in the last few mails, probably the only actionable one\nis the truncated sentence in the trailer iterator header file.\n\nMartin\n"},{"id":"406467","messageId":"20200927075927.GA1286220@coredump.intra.peff.net","threadId":"54293","inReplyTo":"CAN0heSoS837ZCLt8WLRBrthXgOErwz-vXW8Af9v2ZdCcCpWRiw@mail.gmail.com","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T07:59:27Z","receivedAt":"2020-09-27T07:59:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 26, 2020 at 02:31:32PM +0200, Martin Ågren wrote:\n\n> On Fri, 25 Sep 2020 at 09:04, Jeff King wrote:\n> >\n> > In preparation for adding more grouping types, let's\n> > refactor the committer/author grouping code. In particular:\n> >\n> > - the master option is now \"--group\", to make it clear\n> > that the various group types are mutually exclusive. The\n> > \"--committer\" option is an alias for \"--group=committer\".\n> \n> I think this is more than a refactoring of the code. The user interface\n> is also refactored (if that's even the right word). From the subject and\n> the first sentence above, I did not expect this first \"-\" item, nor that\n> the patch would touch Documentation/.\n\nYeah, I agree it's a bit misleading. I've reworded it (especially the\ntitle) to make the intent more clear.\n\n> > + enum {\n> > + SHORTLOG_GROUP_AUTHOR = 0,\n> > + SHORTLOG_GROUP_COMMITTER\n> > + } group;\n> \n> You could reduce the patch noise by adding a trailing comma, see\n> cc0c42975a (\"CodingGuidelines: spell out post-C89 rules\", 2019-07-16).\n> (I do realize that you later redefine all values anyway.)\n\nThanks, that's worth doing. This part of the series was actually from\n2015, so perhaps before we had that guideline. :)\n\n-Peff\n"},{"id":"406468","messageId":"20200927080345.GB1286220@coredump.intra.peff.net","threadId":"54293","inReplyTo":"CAPig+cTfYmNAP_65RDa_fZOyuQEH65HuLs-UYSPT0yJ=s6BftA@mail.gmail.com","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:03:45Z","receivedAt":"2020-09-27T08:03:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2020 at 04:05:04PM -0400, Eric Sunshine wrote:\n\n> > +--group=::\n> > + By default, `shortlog` collects and collates author identities;\n> > + using `--group` will collect other fields.\n> > + `` is one of:\n> > ++\n> > + - `author`, commits are grouped by author (this is the default)\n> > + - `committer`, commits are grouped by committer (the same as `-c`)\n> \n> I had trouble interpreting \"(this is the default)\". It made me think\n> that is optional:\n> \n> --group[=]\n> \n> but that isn't the case at all. Instead, it means that if --group\n> isn't specified, then grouping is done by `author` by default.\n\nRight.\n\n> It also\n> repeats what the general description of --group already says with\n> regard to the default, thus it is redundant to say it again when\n> describing the `author` type. Therefore, perhaps drop \"(this is the\n> default)\" altogether?\n\nOK. I was worried somebody wouldn't quite pick up on the default. We\ncould spell it out as:\n\n - `author`, commits are grouped by author (this is the default if no\n --group is given)\n\nbut that's pretty lengthy. We could also reword the whole thing as:\n\n --group=::\n Collect and collate values of type ``; if no `--group`\n\t option is given, the default is `author`:\n - `author`, commits are grouped by author\n - `committer`, commits are grouped by committer (the same as `-c`)\n\nI'm not sure if that's clearer or not. The first sentence seems a bit\nopaque, but I'm not sure how to improve it.\n\nFor now (and what plan to send out in v2), I'll take your suggestion to\njust drop the phrase.\n\n-Peff\n"},{"id":"406469","messageId":"20200927080626.GC1286220@coredump.intra.peff.net","threadId":"54293","inReplyTo":"CAPig+cS96Y8nMQgazZE-HP4Un2LpgfPF3-63vt8=WJf0c5EuRg@mail.gmail.com","subject":"Re: [PATCH 8/8] shortlog: allow multiple groups to be specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:06:26Z","receivedAt":"2020-09-27T08:06:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2020 at 04:23:39PM -0400, Eric Sunshine wrote:\n\n> > diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\n> > @@ -69,6 +69,11 @@ Shortlog will attempt to parse each trailer value as a `name `\n> > identity. If successful, the mailmap is applied and the email is omitted\n> > unless the `--email` option is specified. If the value cannot be parsed\n> > as an identity, it will be taken literally and completely.\n> > ++\n> > +If `--group` is specified multiple times, commits are counted under each\n> > +value (but again, only once per unique value in that commit). For\n> > +example, `git shortlog --group=author --group=trailer:co-authored-by`\n> > +counts both authors and co-authors.\n> \n> Intuitively, I understand (or hope) that the first use of --group\n> overrides what is otherwise collected by default (authors), however,\n> would there be value in stating this explicitly?\n\nYes. Maybe my \"if no --group is specified\" suggestion from the other\npart of the thread _is_ worth doing then.\n\n> At this point, the documentation for --committer still says:\n> \n> --committer::\n> Collect and show committer identities instead of authors.\n> This is an alias for `--group=committer`.\n> \n> which stops feeling accurate now that --group can be specified more\n> than once. The \"instead of authors\" bit is particularly jarring. I\n> wonder if the first sentence can simply be dropped.\n\nHmm, yeah. I actually kept --committer different at first, having it\noverride any other --group options. But that seemed confusing, too\n(since it's now _almost_ an alias for --group=committer). I think your\nsuggestion works to just explicitly mark it as an alias and nothing.\n\n-Peff\n"},{"id":"406470","messageId":"20200927080846.GD1286220@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927080345.GB1286220@coredump.intra.peff.net","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:08:46Z","receivedAt":"2020-09-27T08:08:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 27, 2020 at 04:03:45AM -0400, Jeff King wrote:\n\n> but that's pretty lengthy. We could also reword the whole thing as:\n> \n> --group=::\n> Collect and collate values of type ``; if no `--group`\n> \t option is given, the default is `author`:\n> - `author`, commits are grouped by author\n> - `committer`, commits are grouped by committer (the same as `-c`)\n> \n> I'm not sure if that's clearer or not. The first sentence seems a bit\n> opaque, but I'm not sure how to improve it.\n> \n> For now (and what plan to send out in v2), I'll take your suggestion to\n> just drop the phrase.\n\nOK, I lied. After reading your other email, I ended up with:\n\n --group=::\n Group commits based on ``. If no `--group` option is\n specified, the default is `author`. `` is one of:\n +\n - `author`, commits are grouped by author\n - `committer`, commits are grouped by committer (the same as `-c`)\n\n -c::\n --committer::\n This is an alias for `--group=committer`.\n\nwhich I think works by itself, and extends nicely to trailers.\n\n-Peff\n"},{"id":"406471","messageId":"20200927082051.GA1292450@coredump.intra.peff.net","threadId":"54293","inReplyTo":"CAN0heSrhmf9-Ep+SWe7Wy-7c3M9-1NFBybSA=6aJ1vDYGTdciA@mail.gmail.com","subject":"Re: [PATCH 3/8] trailer: add interface for iterating over commit trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:20:51Z","receivedAt":"2020-09-27T08:20:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 26, 2020 at 02:39:32PM +0200, Martin Ågren wrote:\n\n> > +/*\n> > + * Initialize \"iter\" in preparation for walking over the trailers in the commit\n> > + * message \"msg\". The \"msg\" pointer must remain valid until the iterator is\n> > + * released.\n> > + *\n> > + * After initializing, we are not yet pointing\n> > + */\n> \n> Truncated sentence. \"... not yet pointing at any trailer\"?\n\nOops. Yes, though I expanded it to:\n\n * After initializing, note that key/val will not yet point to any trailer.\n * Call advance() to parse the first one (if any).\n\nObviously another convention would be:\n\n for (trailer_iterator_init(&iter, msg);\n !trailer_iterator_done();\n trailer_iterator_advance()) {\n }\n\nbut I don't think it matters much either way as long as it's clearly\ndocumented.\n\n-Peff\n"},{"id":"406472","messageId":"CAPig+cT8LRdp6drcEN6bBFprTUxvDjigXtUXqS-VQmN0Nmg+Nw@mail.gmail.com","threadId":"54293","inReplyTo":"20200927080846.GD1286220@coredump.intra.peff.net","subject":"Re: [PATCH 2/8] shortlog: refactor committer/author grouping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-27T08:23:23Z","receivedAt":"2020-09-27T08:23:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Sep 27, 2020 at 4:08 AM Jeff King wrote:\n> OK, I lied. After reading your other email, I ended up with:\n>\n> --group=::\n> Group commits based on ``. If no `--group` option is\n> specified, the default is `author`. `` is one of:\n> +\n> - `author`, commits are grouped by author\n> - `committer`, commits are grouped by committer (the same as `-c`)\n\nSounds good. Makes it much more clear to me.\n"},{"id":"406473","messageId":"20200927082503.GE1286220@coredump.intra.peff.net","threadId":"54293","inReplyTo":"CAN0heSp5ssE1_-0B4VMGcuOaW63EpWXRVp4E4gy_7zVZ1rWkew@mail.gmail.com","subject":"Re: [PATCH 8/8] shortlog: allow multiple groups to be specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:25:03Z","receivedAt":"2020-09-27T08:25:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 26, 2020 at 02:48:44PM +0200, Martin Ågren wrote:\n\n> On Fri, 25 Sep 2020 at 09:09, Jeff King wrote:\n> > +static inline int has_multi_bits(unsigned n)\n> > +{\n> > + return (n & (n - 1)) != 0;\n> > +}\n> \n> There's a HAS_MULTI_BITS macro in git-compat-util.h. I like how you came\n> up with the exact same name. It even makes me wonder if you are actively\n> avoiding it in favor of a re-implementation, but I can't see why.\n\nHeh. I _thought_ we had such a helper, and I even thought I remembered\nthe name. But when I grepped for it, I couldn't come up with it. Silly\n\"-i\". (So I guess it's no surprise that I came up with the same\nimplementation, which is the only good one, and the same name, which I\ndid pull from the back of my mind).\n\nI'll drop this in favor of the macro.\n\n> > enum {\n> > - SHORTLOG_GROUP_AUTHOR = 0,\n> > - SHORTLOG_GROUP_COMMITTER,\n> > - SHORTLOG_GROUP_TRAILER\n> > - } group;\n> > + SHORTLOG_GROUP_AUTHOR = (1 << 0),\n> > + SHORTLOG_GROUP_COMMITTER = (1 << 1),\n> > + SHORTLOG_GROUP_TRAILER = (1 << 2)\n> > + } groups;\n> \n> Coming back to the comment on 2/8, adding a comma at the end wouldn't\n> reduce patch noise here and now, but whenever someone touches this place\n> the next time.\n\nAgreed, will do.\n\n> Of my comments in the last few mails, probably the only actionable one\n> is the truncated sentence in the trailer iterator header file.\n\nI agree none are big, but worth addressing since I was making a re-roll\nanyway. Thanks.\n\n-Peff\n"},{"id":"406474","messageId":"20200927083933.GA2222823@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200925070120.GA3669667@coredump.intra.peff.net","subject":"[PATCH v2 0/8] parsing trailers with shortlog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:39:33Z","receivedAt":"2020-09-27T08:39:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2020 at 03:01:20AM -0400, Jeff King wrote:\n\n> Somebody mentioned at the inclusion summit that it would be nice for\n> mentoring/pairing relationships if we could more easily give credit to\n> multiple authors of a commit. When people ask about adding multiple\n> \"author\" headers, we usually recommend that they use a \"co-authored-by\"\n> trailer. But you can't convince shortlog to count it for anything. :)\n> \n> So this series adds support for counting trailers to shortlog. You can\n> do a number of fun things with it, like:\n\nHere's a re-roll with some cosmetic fixups. All except the first address\nthe points raised by Eric and Martin (thanks, both):\n\n - put the --group= unordered list into a delimited block, so\n that the follow-on paragraph about multiple --group options isn't\n indented with \"trailer\" item\n\n - reword --group docs for clarity\n\n - use existing HAS_MULTI_BITS\n\n - add trailing commas in enum\n\n - complete truncated trailer_iterator comment\n\n - fix misleading \"refactor\" commit message in patch 2\n\nRange diff is below.\n\n [1/8]: shortlog: change \"author\" variables to \"ident\"\n [2/8]: shortlog: add grouping option\n [3/8]: trailer: add interface for iterating over commit trailers\n [4/8]: shortlog: match commit trailers with --group\n [5/8]: shortlog: de-duplicate trailer values\n [6/8]: shortlog: rename parse_stdin_ident()\n [7/8]: shortlog: parse trailer idents\n [8/8]: shortlog: allow multiple groups to be specified\n\n Documentation/git-shortlog.txt | 31 ++++-\n builtin/log.c | 1 +\n builtin/shortlog.c | 213 ++++++++++++++++++++++++++++-----\n shortlog.h | 8 +-\n t/t4201-shortlog.sh | 141 ++++++++++++++++++++++\n trailer.c | 36 ++++++\n trailer.h | 45 ++++++-\n 7 files changed, 444 insertions(+), 31 deletions(-)\n\n1: 675e8cea10 = 1: 3b658cba7c shortlog: change \"author\" variables to \"ident\"\n2: 72d94a2601 ! 2: 3ed8a55597 shortlog: refactor committer/author grouping\n @@ Metadata\n Author: Jeff King \n \n ## Commit message ##\n - shortlog: refactor committer/author grouping\n + shortlog: add grouping option\n \n - In preparation for adding more grouping types, let's\n - refactor the committer/author grouping code. In particular:\n + In preparation for adding more grouping types, let's refactor the\n + committer/author grouping code and add a user-facing option that binds\n + them together. In particular:\n \n - - the master option is now \"--group\", to make it clear\n + - the main option is now \"--group\", to make it clear\n that the various group types are mutually exclusive. The\n \"--committer\" option is an alias for \"--group=committer\".\n \n @@ Documentation/git-shortlog.txt: OPTIONS\n \tEach pretty-printed commit will be rewrapped before it is shown.\n \n +--group=::\n -+\tBy default, `shortlog` collects and collates author identities;\n -+\tusing `--group` will collect other fields.\n -+\t`` is one of:\n ++\tGroup commits based on ``. If no `--group` option is\n ++\tspecified, the default is `author`. `` is one of:\n ++\n -+ - `author`, commits are grouped by author (this is the default)\n ++ - `author`, commits are grouped by author\n + - `committer`, commits are grouped by committer (the same as `-c`)\n +\n -c::\n --committer::\n - \tCollect and show committer identities instead of authors.\n +-\tCollect and show committer identities instead of authors.\n +\tThis is an alias for `--group=committer`.\n \n -w[[,[,]]]::\n @@ shortlog.h: struct shortlog {\n +\n +\tenum {\n +\t\tSHORTLOG_GROUP_AUTHOR = 0,\n -+\t\tSHORTLOG_GROUP_COMMITTER\n ++\t\tSHORTLOG_GROUP_COMMITTER,\n +\t} group;\n \n \tchar *common_repo_prefix;\n3: bec4ec1433 ! 3: d314578bde trailer: add interface for iterating over commit trailers\n @@ trailer.h: void trailer_info_release(struct trailer_info *info);\n + * message \"msg\". The \"msg\" pointer must remain valid until the iterator is\n + * released.\n + *\n -+ * After initializing, we are not yet pointing\n ++ * After initializing, note that key/val will not yet point to any trailer.\n ++ * Call advance() to parse the first one (if any).\n + */\n +void trailer_iterator_init(struct trailer_iterator *iter, const char *msg);\n +\n4: 7e6ee8317f ! 4: fdb1dbd164 shortlog: match commit trailers with --group\n @@ Commit message\n ## Documentation/git-shortlog.txt ##\n @@ Documentation/git-shortlog.txt: OPTIONS\n +\n - - `author`, commits are grouped by author (this is the default)\n + - `author`, commits are grouped by author\n - `committer`, commits are grouped by committer (the same as `-c`)\n + - `trailer:`, the `` is interpreted as a case-insensitive\n + commit message trailer (see linkgit:git-interpret-trailers[1]). For\n @@ builtin/shortlog.c: static int parse_wrap_args(const struct option *opt, const c\n \n ## shortlog.h ##\n @@ shortlog.h: struct shortlog {\n - \n \tenum {\n \t\tSHORTLOG_GROUP_AUTHOR = 0,\n --\t\tSHORTLOG_GROUP_COMMITTER\n -+\t\tSHORTLOG_GROUP_COMMITTER,\n -+\t\tSHORTLOG_GROUP_TRAILER\n + \t\tSHORTLOG_GROUP_COMMITTER,\n ++\t\tSHORTLOG_GROUP_TRAILER,\n \t} group;\n +\tchar *trailer;\n \n5: 8bc6a94239 = 5: 6374604756 shortlog: de-duplicate trailer values\n6: 16294b5d16 = 6: dfabab9957 shortlog: rename parse_stdin_ident()\n7: 82c637c0a2 = 7: bbe87d0e2a shortlog: parse trailer idents\n8: 4cacdecaa0 ! 8: ca782fa92e shortlog: allow multiple groups to be specified\n @@ Commit message\n Signed-off-by: Jeff King \n \n ## Documentation/git-shortlog.txt ##\n +@@ Documentation/git-shortlog.txt: OPTIONS\n + \tGroup commits based on ``. If no `--group` option is\n + \tspecified, the default is `author`. `` is one of:\n + +\n ++--\n + - `author`, commits are grouped by author\n + - `committer`, commits are grouped by committer (the same as `-c`)\n + - `trailer:`, the `` is interpreted as a case-insensitive\n @@ Documentation/git-shortlog.txt: Shortlog will attempt to parse each trailer value as a `name `\n identity. If successful, the mailmap is applied and the email is omitted\n unless the `--email` option is specified. If the value cannot be parsed\n as an identity, it will be taken literally and completely.\n ++--\n ++\n +If `--group` is specified multiple times, commits are counted under each\n +value (but again, only once per unique value in that commit). For\n @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_stdou\n \n \n ## builtin/shortlog.c ##\n -@@ builtin/shortlog.c: static int parse_ident(struct shortlog *log,\n - \treturn 0;\n - }\n - \n -+static inline int has_multi_bits(unsigned n)\n -+{\n -+\treturn (n & (n - 1)) != 0;\n -+}\n -+\n - static void read_from_stdin(struct shortlog *log)\n - {\n - \tstruct strbuf ident = STRBUF_INIT;\n @@ builtin/shortlog.c: static void read_from_stdin(struct shortlog *log)\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n -\tswitch (log->group) {\n -+\tif (has_multi_bits(log->groups))\n ++\tif (HAS_MULTI_BITS(log->groups))\n +\t\tdie(_(\"using multiple --group options with stdin is not supported\"));\n +\n +\tswitch (log->groups) {\n @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit\n -\t\tinsert_one_record(log, ident.buf, oneline_str);\n -\t\tbreak;\n -\tcase SHORTLOG_GROUP_COMMITTER:\n -+\t\tif (!has_multi_bits(log->groups) ||\n ++\t\tif (!HAS_MULTI_BITS(log->groups) ||\n +\t\t !strset_check_and_add(&dups, ident.buf))\n +\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n +\t}\n @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit\n -\tcase SHORTLOG_GROUP_TRAILER:\n -\t\tinsert_records_from_trailers(log, commit, &ctx, oneline_str);\n -\t\tbreak;\n -+\t\tif (!has_multi_bits(log->groups) ||\n ++\t\tif (!HAS_MULTI_BITS(log->groups) ||\n +\t\t !strset_check_and_add(&dups, ident.buf))\n +\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n +\t}\n @@ shortlog.h: struct shortlog {\n \tenum {\n -\t\tSHORTLOG_GROUP_AUTHOR = 0,\n -\t\tSHORTLOG_GROUP_COMMITTER,\n --\t\tSHORTLOG_GROUP_TRAILER\n +-\t\tSHORTLOG_GROUP_TRAILER,\n -\t} group;\n -\tchar *trailer;\n +\t\tSHORTLOG_GROUP_AUTHOR = (1 << 0),\n +\t\tSHORTLOG_GROUP_COMMITTER = (1 << 1),\n -+\t\tSHORTLOG_GROUP_TRAILER = (1 << 2)\n ++\t\tSHORTLOG_GROUP_TRAILER = (1 << 2),\n +\t} groups;\n +\tstruct string_list trailers;\n \n"},{"id":"406475","messageId":"20200927083952.GA2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 1/8] shortlog: change \"author\" variables to \"ident\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:39:52Z","receivedAt":"2020-09-27T08:39:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already match \"committer\", and we're about to start\nmatching more things. Let's use a more neutral variable to\navoid confusion.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex c856c58bb5..edcf2e0d54 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -49,12 +49,12 @@ static int compare_by_list(const void *a1, const void *a2)\n }\n \n static void insert_one_record(struct shortlog *log,\n-\t\t\t const char *author,\n+\t\t\t const char *ident,\n \t\t\t const char *oneline)\n {\n \tstruct string_list_item *item;\n \n-\titem = string_list_insert(&log->list, author);\n+\titem = string_list_insert(&log->list, ident);\n \n \tif (log->summary)\n \t\titem->util = (void *)(UTIL_TO_INT(item) + 1);\n@@ -97,8 +97,8 @@ static void insert_one_record(struct shortlog *log,\n \t}\n }\n \n-static int parse_stdin_author(struct shortlog *log,\n-\t\t\t struct strbuf *out, const char *in)\n+static int parse_stdin_ident(struct shortlog *log,\n+\t\t\t struct strbuf *out, const char *in)\n {\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n@@ -122,18 +122,18 @@ static int parse_stdin_author(struct shortlog *log,\n \n static void read_from_stdin(struct shortlog *log)\n {\n-\tstruct strbuf author = STRBUF_INIT;\n-\tstruct strbuf mapped_author = STRBUF_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n+\tstruct strbuf mapped_ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstatic const char *author_match[2] = { \"Author: \", \"author \" };\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n \tmatch = log->committer ? committer_match : author_match;\n-\twhile (strbuf_getline_lf(&author, stdin) != EOF) {\n+\twhile (strbuf_getline_lf(&ident, stdin) != EOF) {\n \t\tconst char *v;\n-\t\tif (!skip_prefix(author.buf, match[0], &v) &&\n-\t\t !skip_prefix(author.buf, match[1], &v))\n+\t\tif (!skip_prefix(ident.buf, match[0], &v) &&\n+\t\t !skip_prefix(ident.buf, match[1], &v))\n \t\t\tcontinue;\n \t\twhile (strbuf_getline_lf(&oneline, stdin) != EOF &&\n \t\t oneline.len)\n@@ -142,20 +142,20 @@ static void read_from_stdin(struct shortlog *log)\n \t\t !oneline.len)\n \t\t\t; /* discard blanks */\n \n-\t\tstrbuf_reset(&mapped_author);\n-\t\tif (parse_stdin_author(log, &mapped_author, v) < 0)\n+\t\tstrbuf_reset(&mapped_ident);\n+\t\tif (parse_stdin_ident(log, &mapped_ident, v) < 0)\n \t\t\tcontinue;\n \n-\t\tinsert_one_record(log, mapped_author.buf, oneline.buf);\n+\t\tinsert_one_record(log, mapped_ident.buf, oneline.buf);\n \t}\n-\tstrbuf_release(&author);\n-\tstrbuf_release(&mapped_author);\n+\tstrbuf_release(&ident);\n+\tstrbuf_release(&mapped_ident);\n \tstrbuf_release(&oneline);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n-\tstruct strbuf author = STRBUF_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tconst char *fmt;\n@@ -170,17 +170,17 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t(log->email ? \"%cN <%cE>\" : \"%cN\") :\n \t\t(log->email ? \"%aN <%aE>\" : \"%aN\");\n \n-\tformat_commit_message(commit, fmt, &author, &ctx);\n+\tformat_commit_message(commit, fmt, &ident, &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+\tinsert_one_record(log, ident.buf, oneline.len ? oneline.buf : \"\");\n \n-\tstrbuf_release(&author);\n+\tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n }\n \n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406476","messageId":"20200927083959.GB2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 2/8] shortlog: add grouping option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:39:59Z","receivedAt":"2020-09-27T08:40:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In preparation for adding more grouping types, let's refactor the\ncommitter/author grouping code and add a user-facing option that binds\nthem together. In particular:\n\n - the main option is now \"--group\", to make it clear\n that the various group types are mutually exclusive. The\n \"--committer\" option is an alias for \"--group=committer\".\n\n - we keep an enum rather than a binary flag, to prepare\n for more values\n\n - we prefer switch statements to ternary assignment, since\n other group types will need more custom code\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 9 +++++-\n builtin/shortlog.c | 59 +++++++++++++++++++++++++++-------\n shortlog.h | 6 +++-\n t/t4201-shortlog.sh | 5 +++\n 4 files changed, 66 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex a72ea7f7ba..6496d313c1 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -47,9 +47,16 @@ OPTIONS\n \n \tEach pretty-printed commit will be rewrapped before it is shown.\n \n+--group=::\n+\tGroup commits based on ``. If no `--group` option is\n+\tspecified, the default is `author`. `` is one of:\n++\n+ - `author`, commits are grouped by author\n+ - `committer`, commits are grouped by committer (the same as `-c`)\n+\n -c::\n --committer::\n-\tCollect and show committer identities instead of authors.\n+\tThis is an alias for `--group=committer`.\n \n -w[[,[,]]]::\n \tLinewrap the output by wrapping each line at `width`. The first\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex edcf2e0d54..880ce19304 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -129,7 +129,17 @@ static void read_from_stdin(struct shortlog *log)\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n-\tmatch = log->committer ? committer_match : author_match;\n+\tswitch (log->group) {\n+\tcase SHORTLOG_GROUP_AUTHOR:\n+\t\tmatch = author_match;\n+\t\tbreak;\n+\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tmatch = committer_match;\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"unhandled shortlog group\");\n+\t}\n+\n \twhile (strbuf_getline_lf(&ident, stdin) != EOF) {\n \t\tconst char *v;\n \t\tif (!skip_prefix(ident.buf, match[0], &v) &&\n@@ -158,27 +168,36 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n-\tconst char *fmt;\n+\tconst char *oneline_str;\n \n \tctx.fmt = CMIT_FMT_USERFORMAT;\n \tctx.abbrev = log->abbrev;\n \tctx.print_email_subject = 1;\n \tctx.date_mode.type = DATE_NORMAL;\n \tctx.output_encoding = get_log_output_encoding();\n \n-\tfmt = log->committer ?\n-\t\t(log->email ? \"%cN <%cE>\" : \"%cN\") :\n-\t\t(log->email ? \"%aN <%aE>\" : \"%aN\");\n-\n-\tformat_commit_message(commit, fmt, &ident, &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, ident.buf, oneline.len ? oneline.buf : \"\");\n+\toneline_str = oneline.len ? oneline.buf : \"\";\n+\n+\tswitch (log->group) {\n+\tcase SHORTLOG_GROUP_AUTHOR:\n+\t\tformat_commit_message(commit,\n+\t\t\t\t log->email ? \"%aN <%aE>\" : \"%aN\",\n+\t\t\t\t &ident, &ctx);\n+\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t\tbreak;\n+\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tformat_commit_message(commit,\n+\t\t\t\t log->email ? \"%cN <%cE>\" : \"%cN\",\n+\t\t\t\t &ident, &ctx);\n+\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t\tbreak;\n+\t}\n \n \tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n@@ -241,6 +260,21 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static int parse_group_option(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct shortlog *log = opt->value;\n+\n+\tif (unset || !strcasecmp(arg, \"author\"))\n+\t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n+\telse if (!strcasecmp(arg, \"committer\"))\n+\t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n+\telse\n+\t\treturn error(_(\"unknown group type: %s\"), arg);\n+\n+\treturn 0;\n+}\n+\n+\n void shortlog_init(struct shortlog *log)\n {\n \tmemset(log, 0, sizeof(*log));\n@@ -260,8 +294,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n-\t\tOPT_BOOL('c', \"committer\", &log.committer,\n-\t\t\t N_(\"Group by committer rather than author\")),\n+\t\tOPT_SET_INT('c', \"committer\", &log.group,\n+\t\t\t N_(\"Group by committer rather than author\"),\n+\t\t\t SHORTLOG_GROUP_COMMITTER),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\n@@ -271,6 +306,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK_F('w', NULL, &log, N_(\"[,[,]]\"),\n \t\t\tN_(\"Linewrap output\"), PARSE_OPT_OPTARG,\n \t\t\t&parse_wrap_args),\n+\t\tOPT_CALLBACK(0, \"group\", &log, N_(\"field\"),\n+\t\t\tN_(\"Group by field\"), parse_group_option),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/shortlog.h b/shortlog.h\nindex 2fa61c4294..876a52158d 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -15,7 +15,11 @@ struct shortlog {\n \tint in2;\n \tint user_format;\n \tint abbrev;\n-\tint committer;\n+\n+\tenum {\n+\t\tSHORTLOG_GROUP_AUTHOR = 0,\n+\t\tSHORTLOG_GROUP_COMMITTER,\n+\t} group;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex d3a7ce6bbb..65e4468746 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -215,4 +215,9 @@ test_expect_success 'shortlog --committer (external)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--group=committer is the same as --committer' '\n+\tgit shortlog -ns --group=committer HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406477","messageId":"20200927084001.GC2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 3/8] trailer: add interface for iterating over commit trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:01Z","receivedAt":"2020-09-27T08:40:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The trailer code knows how to parse out the trailers and re-format them,\nbut there's no easy way to iterate over the trailers (you can use\ntrailer_info, but you have to then do a bunch of extra parsing).\n\nLet's add an iteration interface that makes this easy to do.\n\nSigned-off-by: Jeff King \n---\n trailer.c | 36 ++++++++++++++++++++++++++++++++++++\n trailer.h | 45 +++++++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 79 insertions(+), 2 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex 68dabc2556..3f7391d793 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1183,3 +1183,39 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg,\n \tformat_trailer_info(out, &info, opts);\n \ttrailer_info_release(&info);\n }\n+\n+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)\n+{\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tstrbuf_init(&iter->key, 0);\n+\tstrbuf_init(&iter->val, 0);\n+\topts.no_divider = 1;\n+\ttrailer_info_get(&iter->info, msg, &opts);\n+\titer->cur = 0;\n+}\n+\n+int trailer_iterator_advance(struct trailer_iterator *iter)\n+{\n+\twhile (iter->cur < iter->info.trailer_nr) {\n+\t\tchar *trailer = iter->info.trailers[iter->cur++];\n+\t\tint separator_pos = find_separator(trailer, separators);\n+\n+\t\tif (separator_pos < 1)\n+\t\t\tcontinue; /* not a real trailer */\n+\n+\t\tstrbuf_reset(&iter->key);\n+\t\tstrbuf_reset(&iter->val);\n+\t\tparse_trailer(&iter->key, &iter->val, NULL,\n+\t\t\t trailer, separator_pos);\n+\t\tunfold_value(&iter->val);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+void trailer_iterator_release(struct trailer_iterator *iter)\n+{\n+\ttrailer_info_release(&iter->info);\n+\tstrbuf_release(&iter->val);\n+\tstrbuf_release(&iter->key);\n+}\ndiff --git a/trailer.h b/trailer.h\nindex 203acf4ee1..cd93e7ddea 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -2,8 +2,7 @@\n #define TRAILER_H\n \n #include \"list.h\"\n-\n-struct strbuf;\n+#include \"strbuf.h\"\n \n enum trailer_where {\n \tWHERE_DEFAULT,\n@@ -103,4 +102,46 @@ void trailer_info_release(struct trailer_info *info);\n void format_trailers_from_commit(struct strbuf *out, const char *msg,\n \t\t\t\t const struct process_trailer_options *opts);\n \n+/*\n+ * An interface for iterating over the trailers found in a particular commit\n+ * message. Use like:\n+ *\n+ * struct trailer_iterator iter;\n+ * trailer_iterator_init(&iter, msg);\n+ * while (trailer_iterator_advance(&iter))\n+ * ... do something with iter.key and iter.val ...\n+ * trailer_iterator_release(&iter);\n+ */\n+struct trailer_iterator {\n+\tstruct strbuf key;\n+\tstruct strbuf val;\n+\n+\t/* private */\n+\tstruct trailer_info info;\n+\tsize_t cur;\n+};\n+\n+/*\n+ * Initialize \"iter\" in preparation for walking over the trailers in the commit\n+ * message \"msg\". The \"msg\" pointer must remain valid until the iterator is\n+ * released.\n+ *\n+ * After initializing, note that key/val will not yet point to any trailer.\n+ * Call advance() to parse the first one (if any).\n+ */\n+void trailer_iterator_init(struct trailer_iterator *iter, const char *msg);\n+\n+/*\n+ * Advance to the next trailer of the iterator. Returns 0 if there is no such\n+ * trailer, and 1 otherwise. The key and value of the trailer can be\n+ * fetched from the iter->key and iter->value fields (which are valid\n+ * only until the next advance).\n+ */\n+int trailer_iterator_advance(struct trailer_iterator *iter);\n+\n+/*\n+ * Release all resources associated with the trailer iteration.\n+ */\n+void trailer_iterator_release(struct trailer_iterator *iter);\n+\n #endif /* TRAILER_H */\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406478","messageId":"20200927084004.GD2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 4/8] shortlog: match commit trailers with --group","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:04Z","receivedAt":"2020-09-27T08:40:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If a project uses commit trailers, this patch lets you use\nshortlog to see who is performing each action. For example,\nrunning:\n\n git shortlog -ns --group=trailer:reviewed-by\n\nin git.git shows who has reviewed. You can even use a custom\nformat to see things like who has helped whom:\n\n git shortlog --format=\"...helped %an (%ad)\" \\\n --group=trailer:helped-by\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 13 ++++++++++\n builtin/shortlog.c | 44 +++++++++++++++++++++++++++++++++-\n shortlog.h | 2 ++\n t/t4201-shortlog.sh | 14 +++++++++++\n 4 files changed, 72 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 6496d313c1..edd6cda58a 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -53,6 +53,19 @@ OPTIONS\n +\n - `author`, commits are grouped by author\n - `committer`, commits are grouped by committer (the same as `-c`)\n+ - `trailer:`, the `` is interpreted as a case-insensitive\n+ commit message trailer (see linkgit:git-interpret-trailers[1]). For\n+ example, if your project uses `Reviewed-by` trailers, you might want\n+ to see who has been reviewing with\n+ `git shortlog -ns --group=trailer:reviewed-by`.\n++\n+Note that commits that do not include the trailer will not be counted.\n+Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n+be counted more than once.\n++\n+The contents of each trailer value are taken literally and completely.\n+No mailmap is applied, and the `-e` option has no effect (if the trailer\n+contains a username and email, they are both always shown).\n \n -c::\n --committer::\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 880ce19304..e1d9ee909f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -9,6 +9,7 @@\n #include \"mailmap.h\"\n #include \"shortlog.h\"\n #include \"parse-options.h\"\n+#include \"trailer.h\"\n \n static char const * const shortlog_usage[] = {\n \tN_(\"git shortlog [] [] [[--] ...]\"),\n@@ -136,6 +137,8 @@ static void read_from_stdin(struct shortlog *log)\n \tcase SHORTLOG_GROUP_COMMITTER:\n \t\tmatch = committer_match;\n \t\tbreak;\n+\tcase SHORTLOG_GROUP_TRAILER:\n+\t\tdie(_(\"using --group=trailer with stdin is not supported\"));\n \tdefault:\n \t\tBUG(\"unhandled shortlog group\");\n \t}\n@@ -163,6 +166,37 @@ static void read_from_stdin(struct shortlog *log)\n \tstrbuf_release(&oneline);\n }\n \n+static void insert_records_from_trailers(struct shortlog *log,\n+\t\t\t\t\t struct commit *commit,\n+\t\t\t\t\t struct pretty_print_context *ctx,\n+\t\t\t\t\t const char *oneline)\n+{\n+\tstruct trailer_iterator iter;\n+\tconst char *commit_buffer, *body;\n+\n+\t/*\n+\t * Using format_commit_message(\"%B\") would be simpler here, but\n+\t * this saves us copying the message.\n+\t */\n+\tcommit_buffer = logmsg_reencode(commit, NULL, ctx->output_encoding);\n+\tbody = strstr(commit_buffer, \"\\n\\n\");\n+\tif (!body)\n+\t\treturn;\n+\n+\ttrailer_iterator_init(&iter, body);\n+\twhile (trailer_iterator_advance(&iter)) {\n+\t\tconst char *value = iter.val.buf;\n+\n+\t\tif (strcasecmp(iter.key.buf, log->trailer))\n+\t\t\tcontinue;\n+\n+\t\tinsert_one_record(log, value, oneline);\n+\t}\n+\ttrailer_iterator_release(&iter);\n+\n+\tunuse_commit_buffer(commit, commit_buffer);\n+}\n+\n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n \tstruct strbuf ident = STRBUF_INIT;\n@@ -197,6 +231,9 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t\t\t\t &ident, &ctx);\n \t\tinsert_one_record(log, ident.buf, oneline_str);\n \t\tbreak;\n+\tcase SHORTLOG_GROUP_TRAILER:\n+\t\tinsert_records_from_trailers(log, commit, &ctx, oneline_str);\n+\t\tbreak;\n \t}\n \n \tstrbuf_release(&ident);\n@@ -263,12 +300,17 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)\n static int parse_group_option(const struct option *opt, const char *arg, int unset)\n {\n \tstruct shortlog *log = opt->value;\n+\tconst char *field;\n \n \tif (unset || !strcasecmp(arg, \"author\"))\n \t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n \telse if (!strcasecmp(arg, \"committer\"))\n \t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n-\telse\n+\telse if (skip_prefix(arg, \"trailer:\", &field)) {\n+\t\tlog->group = SHORTLOG_GROUP_TRAILER;\n+\t\tfree(log->trailer);\n+\t\tlog->trailer = xstrdup(field);\n+\t} else\n \t\treturn error(_(\"unknown group type: %s\"), arg);\n \n \treturn 0;\ndiff --git a/shortlog.h b/shortlog.h\nindex 876a52158d..89c2dbc5e6 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -19,7 +19,9 @@ struct shortlog {\n \tenum {\n \t\tSHORTLOG_GROUP_AUTHOR = 0,\n \t\tSHORTLOG_GROUP_COMMITTER,\n+\t\tSHORTLOG_GROUP_TRAILER,\n \t} group;\n+\tchar *trailer;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 65e4468746..e97d891a71 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -220,4 +220,18 @@ test_expect_success '--group=committer is the same as --committer' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog --group=trailer:signed-off-by' '\n+\tgit commit --allow-empty -m foo -s &&\n+\tGIT_COMMITTER_NAME=\"SOB One\" \\\n+\tGIT_COMMITTER_EMAIL=sob@example.com \\\n+\t\tgit commit --allow-empty -m foo -s &&\n+\tgit commit --allow-empty --amend --no-edit -s &&\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter \n+\t 1\tSOB One \n+\tEOF\n+\tgit shortlog -ns --group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406479","messageId":"20200927084007.GE2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 5/8] shortlog: de-duplicate trailer values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:07Z","receivedAt":"2020-09-27T08:40:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current documentation is vague about what happens with\n--group=trailer:signed-off-by when we see a commit with:\n\n Signed-off-by: One\n Signed-off-by: Two\n Signed-off-by: One\n\nWe clearly should credit both \"One\" and \"Two\", but should \"One\" get\ncredited twice? The current code does so, but mostly because that was\nthe easiest thing to do. It's probably more useful to count each commit\nat most once. This will become especially important when we allow\nvalues from multiple sources in a future patch.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 3 +-\n builtin/shortlog.c | 58 ++++++++++++++++++++++++++++++++++\n t/t4201-shortlog.sh | 28 ++++++++++++++++\n 3 files changed, 88 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex edd6cda58a..9e94613e13 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -61,7 +61,8 @@ OPTIONS\n +\n Note that commits that do not include the trailer will not be counted.\n Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n-be counted more than once.\n+be counted more than once (but only once per unique trailer value in\n+that commit).\n +\n The contents of each trailer value are taken literally and completely.\n No mailmap is applied, and the `-e` option has no effect (if the trailer\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e1d9ee909f..d2d8103dd3 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -166,13 +166,68 @@ static void read_from_stdin(struct shortlog *log)\n \tstrbuf_release(&oneline);\n }\n \n+struct strset_item {\n+\tstruct hashmap_entry ent;\n+\tchar value[FLEX_ARRAY];\n+};\n+\n+struct strset {\n+\tstruct hashmap map;\n+};\n+\n+#define STRSET_INIT { { NULL } }\n+\n+static int strset_item_hashcmp(const void *hash_data,\n+\t\t\t const struct hashmap_entry *entry,\n+\t\t\t const struct hashmap_entry *entry_or_key,\n+\t\t\t const void *keydata)\n+{\n+\tconst struct strset_item *a, *b;\n+\n+\ta = container_of(entry, const struct strset_item, ent);\n+\tif (keydata)\n+\t\treturn strcmp(a->value, keydata);\n+\n+\tb = container_of(entry_or_key, const struct strset_item, ent);\n+\treturn strcmp(a->value, b->value);\n+}\n+\n+/*\n+ * Adds \"str\" to the set if it was not already present; returns true if it was\n+ * already there.\n+ */\n+static int strset_check_and_add(struct strset *ss, const char *str)\n+{\n+\tunsigned int hash = strhash(str);\n+\tstruct strset_item *item;\n+\n+\tif (!ss->map.table)\n+\t\thashmap_init(&ss->map, strset_item_hashcmp, NULL, 0);\n+\n+\tif (hashmap_get_from_hash(&ss->map, hash, str))\n+\t\treturn 1;\n+\n+\tFLEX_ALLOC_STR(item, value, str);\n+\thashmap_entry_init(&item->ent, hash);\n+\thashmap_add(&ss->map, &item->ent);\n+\treturn 0;\n+}\n+\n+static void strset_clear(struct strset *ss)\n+{\n+\tif (!ss->map.table)\n+\t\treturn;\n+\thashmap_free_entries(&ss->map, struct strset_item, ent);\n+}\n+\n static void insert_records_from_trailers(struct shortlog *log,\n \t\t\t\t\t struct commit *commit,\n \t\t\t\t\t struct pretty_print_context *ctx,\n \t\t\t\t\t const char *oneline)\n {\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n+\tstruct strset dups = STRSET_INIT;\n \n \t/*\n \t * Using format_commit_message(\"%B\") would be simpler here, but\n@@ -190,10 +245,13 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\tif (strcasecmp(iter.key.buf, log->trailer))\n \t\t\tcontinue;\n \n+\t\tif (strset_check_and_add(&dups, value))\n+\t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n+\tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\n \ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex e97d891a71..83dbbc44e8 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -234,4 +234,32 @@ test_expect_success 'shortlog --group=trailer:signed-off-by' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject one\n+\n+\tthis message has two distinct values, plus a repeat\n+\n+\tRepeated-trailer: Foo\n+\tRepeated-trailer: Bar\n+\tRepeated-trailer: Foo\n+\tEOF\n+\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject two\n+\n+\tsimilar to the previous, but without the second distinct value\n+\n+\tRepeated-trailer: Foo\n+\tRepeated-trailer: Foo\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\t 2\tFoo\n+\t 1\tBar\n+\tEOF\n+\tgit shortlog -ns --group=trailer:repeated-trailer -2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406480","messageId":"20200927084009.GF2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 6/8] shortlog: rename parse_stdin_ident()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:09Z","receivedAt":"2020-09-27T08:40:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function is actually useful for parsing any identity, whether from\nstdin or not. We'll need it for handling trailers.\n\nSigned-off-by: Jeff King \n---\n builtin/shortlog.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex d2d8103dd3..e6f4faec7c 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -98,8 +98,8 @@ static void insert_one_record(struct shortlog *log,\n \t}\n }\n \n-static int parse_stdin_ident(struct shortlog *log,\n-\t\t\t struct strbuf *out, const char *in)\n+static int parse_ident(struct shortlog *log,\n+\t\t struct strbuf *out, const char *in)\n {\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n@@ -156,7 +156,7 @@ static void read_from_stdin(struct shortlog *log)\n \t\t\t; /* discard blanks */\n \n \t\tstrbuf_reset(&mapped_ident);\n-\t\tif (parse_stdin_ident(log, &mapped_ident, v) < 0)\n+\t\tif (parse_ident(log, &mapped_ident, v) < 0)\n \t\t\tcontinue;\n \n \t\tinsert_one_record(log, mapped_ident.buf, oneline.buf);\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406481","messageId":"20200927084011.GG2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 7/8] shortlog: parse trailer idents","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:11Z","receivedAt":"2020-09-27T08:40:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Trailers don't necessarily contain name/email identity values, so\nshortlog has so far treated them as opaque strings. However, since many\ntrailers do contain identities, it's useful to treat them as such when\nthey can be parsed. That lets \"-e\" work as usual, as well as mailmap.\n\nWhen they can't be parsed, we'll continue with the old behavior of\ntreating them as a single string (there's no new test for that here,\nsince the existing tests cover a trailer like this).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 7 ++++---\n builtin/shortlog.c | 6 ++++++\n t/t4201-shortlog.sh | 20 ++++++++++++++++++++\n 3 files changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 9e94613e13..3db0db2da0 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -64,9 +64,10 @@ Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n be counted more than once (but only once per unique trailer value in\n that commit).\n +\n-The contents of each trailer value are taken literally and completely.\n-No mailmap is applied, and the `-e` option has no effect (if the trailer\n-contains a username and email, they are both always shown).\n+Shortlog will attempt to parse each trailer value as a `name `\n+identity. If successful, the mailmap is applied and the email is omitted\n+unless the `--email` option is specified. If the value cannot be parsed\n+as an identity, it will be taken literally and completely.\n \n -c::\n --committer::\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e6f4faec7c..28133aec68 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -228,6 +228,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n \tstruct strset dups = STRSET_INIT;\n+\tstruct strbuf ident = STRBUF_INIT;\n \n \t/*\n \t * Using format_commit_message(\"%B\") would be simpler here, but\n@@ -245,12 +246,17 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\tif (strcasecmp(iter.key.buf, log->trailer))\n \t\t\tcontinue;\n \n+\t\tstrbuf_reset(&ident);\n+\t\tif (!parse_ident(log, &ident, value))\n+\t\t\tvalue = ident.buf;\n+\n \t\tif (strset_check_and_add(&dups, value))\n \t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n+\tstrbuf_release(&ident);\n \tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 83dbbc44e8..a62ee9ed55 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -230,10 +230,30 @@ test_expect_success 'shortlog --group=trailer:signed-off-by' '\n \t 2\tC O Mitter \n \t 1\tSOB One \n \tEOF\n+\tgit shortlog -nse --group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'trailer idents are split' '\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter\n+\t 1\tSOB One\n+\tEOF\n \tgit shortlog -ns --group=trailer:signed-off-by HEAD >actual &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'trailer idents are mailmapped' '\n+\tcat >expect <<-\\EOF &&\n+\t 2\tC O Mitter\n+\t 1\tAnother Name\n+\tEOF\n+\techo \"Another Name \" >mail.map &&\n+\tgit -c mailmap.file=mail.map shortlog -ns \\\n+\t\t--group=trailer:signed-off-by HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n \tgit commit --allow-empty -F - <<-\\EOF &&\n \tsubject one\n-- \n2.28.0.1127.ga65787d918\n\n"},{"id":"406482","messageId":"20200927084015.GH2465761@coredump.intra.peff.net","threadId":"54293","inReplyTo":"20200927083933.GA2222823@coredump.intra.peff.net","subject":"[PATCH v2 8/8] shortlog: allow multiple groups to be specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-27T08:40:15Z","receivedAt":"2020-09-27T08:40:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that shortlog supports reading from trailers, it can be useful to\ncombine counts from multiple trailers, or between trailers and authors.\nThis can be done manually by post-processing the output from multiple\nruns, but it's non-trivial to make sure that each name/commit pair is\ncounted only once.\n\nThis patch teaches shortlog to accept multiple --group options on the\ncommand line, and pull data from all of them. That makes it possible to\nrun:\n\n git shortlog -ns --group=author --group=trailer:co-authored-by\n\nto get a shortlog that counts authors and co-authors equally.\n\nThe implementation is mostly straightforward. The \"group\" enum becomes a\nbitfield, and the trailer key becomes a list. I didn't bother\nimplementing the multi-group semantics for reading from stdin. It would\nbe possible to do, but the existing matching code makes it awkward, and\nI doubt anybody cares.\n\nThe duplicate suppression we used for trailers now covers authors and\ncommitters as well (though in non-trailer single-group mode we can skip\nthe hash insertion and lookup, since we only see one value per commit).\n\nThere is one subtlety: we now care about the case when no group bit is\nset (in which case we default to showing the author). The caller in\nbuiltin/log.c needs to be adapted to ask explicitly for authors, rather\nthan relying on shortlog_init(). It would be possible with some\ngymnastics to make this keep working as-is, but it's not worth it for a\nsingle caller.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-shortlog.txt | 7 ++++\n builtin/log.c | 1 +\n builtin/shortlog.c | 64 ++++++++++++++++++-----------\n shortlog.h | 10 ++---\n t/t4201-shortlog.sh | 74 ++++++++++++++++++++++++++++++++++\n 5 files changed, 127 insertions(+), 29 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 3db0db2da0..fd93cd41e9 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -51,6 +51,7 @@ OPTIONS\n \tGroup commits based on ``. If no `--group` option is\n \tspecified, the default is `author`. `` is one of:\n +\n+--\n - `author`, commits are grouped by author\n - `committer`, commits are grouped by committer (the same as `-c`)\n - `trailer:`, the `` is interpreted as a case-insensitive\n@@ -68,6 +69,12 @@ Shortlog will attempt to parse each trailer value as a `name `\n identity. If successful, the mailmap is applied and the email is omitted\n unless the `--email` option is specified. If the value cannot be parsed\n as an identity, it will be taken literally and completely.\n+--\n++\n+If `--group` is specified multiple times, commits are counted under each\n+value (but again, only once per unique value in that commit). For\n+example, `git shortlog --group=author --group=trailer:co-authored-by`\n+counts both authors and co-authors.\n \n -c::\n --committer::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b8824d898f..7f27e9eca1 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1195,6 +1195,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tlog.in1 = 2;\n \tlog.in2 = 4;\n \tlog.file = rev->diffopt.file;\n+\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n \tfor (i = 0; i < nr; i++)\n \t\tshortlog_add_commit(&log, list[i]);\n \ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 28133aec68..0a5c4968f6 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -130,7 +130,10 @@ static void read_from_stdin(struct shortlog *log)\n \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n \tconst char **match;\n \n-\tswitch (log->group) {\n+\tif (HAS_MULTI_BITS(log->groups))\n+\t\tdie(_(\"using multiple --group options with stdin is not supported\"));\n+\n+\tswitch (log->groups) {\n \tcase SHORTLOG_GROUP_AUTHOR:\n \t\tmatch = author_match;\n \t\tbreak;\n@@ -221,13 +224,13 @@ static void strset_clear(struct strset *ss)\n }\n \n static void insert_records_from_trailers(struct shortlog *log,\n+\t\t\t\t\t struct strset *dups,\n \t\t\t\t\t struct commit *commit,\n \t\t\t\t\t struct pretty_print_context *ctx,\n \t\t\t\t\t const char *oneline)\n {\n \tstruct trailer_iterator iter;\n \tconst char *commit_buffer, *body;\n-\tstruct strset dups = STRSET_INIT;\n \tstruct strbuf ident = STRBUF_INIT;\n \n \t/*\n@@ -243,28 +246,28 @@ static void insert_records_from_trailers(struct shortlog *log,\n \twhile (trailer_iterator_advance(&iter)) {\n \t\tconst char *value = iter.val.buf;\n \n-\t\tif (strcasecmp(iter.key.buf, log->trailer))\n+\t\tif (!string_list_has_string(&log->trailers, iter.key.buf))\n \t\t\tcontinue;\n \n \t\tstrbuf_reset(&ident);\n \t\tif (!parse_ident(log, &ident, value))\n \t\t\tvalue = ident.buf;\n \n-\t\tif (strset_check_and_add(&dups, value))\n+\t\tif (strset_check_and_add(dups, value))\n \t\t\tcontinue;\n \t\tinsert_one_record(log, value, oneline);\n \t}\n \ttrailer_iterator_release(&iter);\n \n \tstrbuf_release(&ident);\n-\tstrset_clear(&dups);\n \tunuse_commit_buffer(commit, commit_buffer);\n }\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n {\n \tstruct strbuf ident = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n+\tstruct strset dups = STRSET_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tconst char *oneline_str;\n \n@@ -282,24 +285,29 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \t}\n \toneline_str = oneline.len ? oneline.buf : \"\";\n \n-\tswitch (log->group) {\n-\tcase SHORTLOG_GROUP_AUTHOR:\n+\tif (log->groups & SHORTLOG_GROUP_AUTHOR) {\n+\t\tstrbuf_reset(&ident);\n \t\tformat_commit_message(commit,\n \t\t\t\t log->email ? \"%aN <%aE>\" : \"%aN\",\n \t\t\t\t &ident, &ctx);\n-\t\tinsert_one_record(log, ident.buf, oneline_str);\n-\t\tbreak;\n-\tcase SHORTLOG_GROUP_COMMITTER:\n+\t\tif (!HAS_MULTI_BITS(log->groups) ||\n+\t\t !strset_check_and_add(&dups, ident.buf))\n+\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t}\n+\tif (log->groups & SHORTLOG_GROUP_COMMITTER) {\n+\t\tstrbuf_reset(&ident);\n \t\tformat_commit_message(commit,\n \t\t\t\t log->email ? \"%cN <%cE>\" : \"%cN\",\n \t\t\t\t &ident, &ctx);\n-\t\tinsert_one_record(log, ident.buf, oneline_str);\n-\t\tbreak;\n-\tcase SHORTLOG_GROUP_TRAILER:\n-\t\tinsert_records_from_trailers(log, commit, &ctx, oneline_str);\n-\t\tbreak;\n+\t\tif (!HAS_MULTI_BITS(log->groups) ||\n+\t\t !strset_check_and_add(&dups, ident.buf))\n+\t\t\tinsert_one_record(log, ident.buf, oneline_str);\n+\t}\n+\tif (log->groups & SHORTLOG_GROUP_TRAILER) {\n+\t\tinsert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);\n \t}\n \n+\tstrset_clear(&dups);\n \tstrbuf_release(&ident);\n \tstrbuf_release(&oneline);\n }\n@@ -366,14 +374,16 @@ static int parse_group_option(const struct option *opt, const char *arg, int uns\n \tstruct shortlog *log = opt->value;\n \tconst char *field;\n \n-\tif (unset || !strcasecmp(arg, \"author\"))\n-\t\tlog->group = SHORTLOG_GROUP_AUTHOR;\n+\tif (unset) {\n+\t\tlog->groups = 0;\n+\t\tstring_list_clear(&log->trailers, 0);\n+\t} else if (!strcasecmp(arg, \"author\"))\n+\t\tlog->groups |= SHORTLOG_GROUP_AUTHOR;\n \telse if (!strcasecmp(arg, \"committer\"))\n-\t\tlog->group = SHORTLOG_GROUP_COMMITTER;\n+\t\tlog->groups |= SHORTLOG_GROUP_COMMITTER;\n \telse if (skip_prefix(arg, \"trailer:\", &field)) {\n-\t\tlog->group = SHORTLOG_GROUP_TRAILER;\n-\t\tfree(log->trailer);\n-\t\tlog->trailer = xstrdup(field);\n+\t\tlog->groups |= SHORTLOG_GROUP_TRAILER;\n+\t\tstring_list_append(&log->trailers, field);\n \t} else\n \t\treturn error(_(\"unknown group type: %s\"), arg);\n \n@@ -391,6 +401,8 @@ void shortlog_init(struct shortlog *log)\n \tlog->wrap = DEFAULT_WRAPLEN;\n \tlog->in1 = DEFAULT_INDENT1;\n \tlog->in2 = DEFAULT_INDENT2;\n+\tlog->trailers.strdup_strings = 1;\n+\tlog->trailers.cmp = strcasecmp;\n }\n \n int cmd_shortlog(int argc, const char **argv, const char *prefix)\n@@ -400,9 +412,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n-\t\tOPT_SET_INT('c', \"committer\", &log.group,\n-\t\t\t N_(\"Group by committer rather than author\"),\n-\t\t\t SHORTLOG_GROUP_COMMITTER),\n+\t\tOPT_BIT('c', \"committer\", &log.groups,\n+\t\t\tN_(\"Group by committer rather than author\"),\n+\t\t\tSHORTLOG_GROUP_COMMITTER),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\n@@ -454,6 +466,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tlog.abbrev = rev.abbrev;\n \tlog.file = rev.diffopt.file;\n \n+\tif (!log.groups)\n+\t\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n+\tstring_list_sort(&log.trailers);\n+\n \t/* assume HEAD if from a tty */\n \tif (!nongit && !rev.pending.nr && isatty(0))\n \t\tadd_head_to_pending(&rev);\ndiff --git a/shortlog.h b/shortlog.h\nindex 89c2dbc5e6..64be879b24 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -17,11 +17,11 @@ struct shortlog {\n \tint abbrev;\n \n \tenum {\n-\t\tSHORTLOG_GROUP_AUTHOR = 0,\n-\t\tSHORTLOG_GROUP_COMMITTER,\n-\t\tSHORTLOG_GROUP_TRAILER,\n-\t} group;\n-\tchar *trailer;\n+\t\tSHORTLOG_GROUP_AUTHOR = (1 << 0),\n+\t\tSHORTLOG_GROUP_COMMITTER = (1 << 1),\n+\t\tSHORTLOG_GROUP_TRAILER = (1 << 2),\n+\t} groups;\n+\tstruct string_list trailers;\n \n \tchar *common_repo_prefix;\n \tint email;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex a62ee9ed55..3d5c4a2086 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -282,4 +282,78 @@ test_expect_success 'shortlog de-duplicates trailers in a single commit' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shortlog can match multiple groups' '\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject one\n+\n+\tthis has two trailers that are distinct from the author; it will count\n+\t3 times in the output\n+\n+\tSome-trailer: User A \n+\tAnother-trailer: User B \n+\tEOF\n+\n+\tgit commit --allow-empty -F - <<-\\EOF &&\n+\tsubject two\n+\n+\tthis one has two trailers, one of which is a duplicate with the author;\n+\tit will only be counted once for them\n+\n+\tAnother-trailer: A U Thor \n+\tSome-trailer: User B \n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\t 2\tA U Thor\n+\t 2\tUser B\n+\t 1\tUser A\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=author \\\n+\t\t--group=trailer:some-trailer \\\n+\t\t--group=trailer:another-trailer \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'set up option selection tests' '\n+\tgit commit --allow-empty -F - <<-\\EOF\n+\tsubject\n+\n+\tbody\n+\n+\tTrailer-one: value-one\n+\tTrailer-two: value-two\n+\tEOF\n+'\n+\n+test_expect_success '--no-group resets group list to author' '\n+\tcat >expect <<-\\EOF &&\n+\t 1\tA U Thor\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=committer \\\n+\t\t--group=trailer:trailer-one \\\n+\t\t--no-group \\\n+\t\t-1 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-group resets trailer list' '\n+\tcat >expect <<-\\EOF &&\n+\t 1\tvalue-two\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=trailer:trailer-one \\\n+\t\t--no-group \\\n+\t\t--group=trailer:trailer-two \\\n+\t\t-1 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin with multiple groups reports error' '\n+\tgit log >log &&\n+\ttest_must_fail git shortlog --group=author --group=committer wrote:\n>\n> Here's a re-roll with some cosmetic fixups. All except the first address\n> the points raised by Eric and Martin (thanks, both):\n>\n> - put the --group= unordered list into a delimited block, so\n> that the follow-on paragraph about multiple --group options isn't\n> indented with \"trailer\" item\n\nYep, that makes sense. The result looks good.\n\nThis v2 addresses my comments on v1 and, FWIW, looks good to me.\n\nMartin\n"},{"id":"406508","messageId":"xmqqpn67ni3i.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927083952.GA2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/8] shortlog: change \"author\" variables to \"ident\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-27T19:18:57Z","receivedAt":"2020-09-27T19:19:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> We already match \"committer\", and we're about to start\n> matching more things. Let's use a more neutral variable to\n> avoid confusion.\n\nLooks good at least when seen alone.\n\nIt is curious how we are going to handle less structured nature of\ntrailers. The author and committer header fields are required to be\nin good shape, but it is perfectly fine for *-by lines to be just\nhuman-readable strings (e.g. without ).\n\nWe'll see.\n\nThanks.\n\n> Signed-off-by: Jeff King \n> ---\n> builtin/shortlog.c | 36 ++++++++++++++++++------------------\n> 1 file changed, 18 insertions(+), 18 deletions(-)\n>\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index c856c58bb5..edcf2e0d54 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -49,12 +49,12 @@ static int compare_by_list(const void *a1, const void *a2)\n> }\n> \n> static void insert_one_record(struct shortlog *log,\n> -\t\t\t const char *author,\n> +\t\t\t const char *ident,\n> \t\t\t const char *oneline)\n> {\n> \tstruct string_list_item *item;\n> \n> -\titem = string_list_insert(&log->list, author);\n> +\titem = string_list_insert(&log->list, ident);\n> \n> \tif (log->summary)\n> \t\titem->util = (void *)(UTIL_TO_INT(item) + 1);\n> @@ -97,8 +97,8 @@ static void insert_one_record(struct shortlog *log,\n> \t}\n> }\n> \n> -static int parse_stdin_author(struct shortlog *log,\n> -\t\t\t struct strbuf *out, const char *in)\n> +static int parse_stdin_ident(struct shortlog *log,\n> +\t\t\t struct strbuf *out, const char *in)\n> {\n> \tconst char *mailbuf, *namebuf;\n> \tsize_t namelen, maillen;\n> @@ -122,18 +122,18 @@ static int parse_stdin_author(struct shortlog *log,\n> \n> static void read_from_stdin(struct shortlog *log)\n> {\n> -\tstruct strbuf author = STRBUF_INIT;\n> -\tstruct strbuf mapped_author = STRBUF_INIT;\n> +\tstruct strbuf ident = STRBUF_INIT;\n> +\tstruct strbuf mapped_ident = STRBUF_INIT;\n> \tstruct strbuf oneline = STRBUF_INIT;\n> \tstatic const char *author_match[2] = { \"Author: \", \"author \" };\n> \tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n> \tconst char **match;\n> \n> \tmatch = log->committer ? committer_match : author_match;\n> -\twhile (strbuf_getline_lf(&author, stdin) != EOF) {\n> +\twhile (strbuf_getline_lf(&ident, stdin) != EOF) {\n> \t\tconst char *v;\n> -\t\tif (!skip_prefix(author.buf, match[0], &v) &&\n> -\t\t !skip_prefix(author.buf, match[1], &v))\n> +\t\tif (!skip_prefix(ident.buf, match[0], &v) &&\n> +\t\t !skip_prefix(ident.buf, match[1], &v))\n> \t\t\tcontinue;\n> \t\twhile (strbuf_getline_lf(&oneline, stdin) != EOF &&\n> \t\t oneline.len)\n> @@ -142,20 +142,20 @@ static void read_from_stdin(struct shortlog *log)\n> \t\t !oneline.len)\n> \t\t\t; /* discard blanks */\n> \n> -\t\tstrbuf_reset(&mapped_author);\n> -\t\tif (parse_stdin_author(log, &mapped_author, v) < 0)\n> +\t\tstrbuf_reset(&mapped_ident);\n> +\t\tif (parse_stdin_ident(log, &mapped_ident, v) < 0)\n> \t\t\tcontinue;\n> \n> -\t\tinsert_one_record(log, mapped_author.buf, oneline.buf);\n> +\t\tinsert_one_record(log, mapped_ident.buf, oneline.buf);\n> \t}\n> -\tstrbuf_release(&author);\n> -\tstrbuf_release(&mapped_author);\n> +\tstrbuf_release(&ident);\n> +\tstrbuf_release(&mapped_ident);\n> \tstrbuf_release(&oneline);\n> }\n> \n> void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n> {\n> -\tstruct strbuf author = STRBUF_INIT;\n> +\tstruct strbuf ident = STRBUF_INIT;\n> \tstruct strbuf oneline = STRBUF_INIT;\n> \tstruct pretty_print_context ctx = {0};\n> \tconst char *fmt;\n> @@ -170,17 +170,17 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n> \t\t(log->email ? \"%cN <%cE>\" : \"%cN\") :\n> \t\t(log->email ? \"%aN <%aE>\" : \"%aN\");\n> \n> -\tformat_commit_message(commit, fmt, &author, &ctx);\n> +\tformat_commit_message(commit, fmt, &ident, &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> +\tinsert_one_record(log, ident.buf, oneline.len ? oneline.buf : \"\");\n> \n> -\tstrbuf_release(&author);\n> +\tstrbuf_release(&ident);\n> \tstrbuf_release(&oneline);\n> }\n"},{"id":"406510","messageId":"xmqqlfgvngkw.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927084004.GD2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/8] shortlog: match commit trailers with --group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-27T19:51:43Z","receivedAt":"2020-09-27T19:51:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If a project uses commit trailers, this patch lets you use\n> shortlog to see who is performing each action. For example,\n> running:\n>\n> git shortlog -ns --group=trailer:reviewed-by\n>\n> in git.git shows who has reviewed. You can even use a custom\n> format to see things like who has helped whom:\n>\n> git shortlog --format=\"...helped %an (%ad)\" \\\n> --group=trailer:helped-by\n\nThat's a cute example.\n\n> +Note that commits that do not include the trailer will not be counted.\n\nUnderstandable.\n\n> +Likewise, commits with multiple trailers (e.g., multiple signoffs) may\n> +be counted more than once.\n\nSolicits a \"is it desirable, or is it just too hard to dedupe?\" response.\n\n> +The contents of each trailer value are taken literally and completely.\n> +No mailmap is applied, and the `-e` option has no effect (if the trailer\n> +contains a username and email, they are both always shown).\n\nOK. Some users may find that not quite satisfying, though.\n\nBut I have a suspicion that the above will be refined in later\nsteps? It would have been nicer to see that mentioned in the\nproposed log message (e.g. \"this step gives the minimum basics and\nrough edges like X and Y will be refined with later patches\").\n\nThanks.\n"},{"id":"406512","messageId":"xmqqh7rjnf42.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927084007.GE2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 5/8] shortlog: de-duplicate trailer values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-27T20:23:25Z","receivedAt":"2020-09-27T20:23:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The current documentation is vague about what happens with\n> --group=trailer:signed-off-by when we see a commit with:\n>\n> Signed-off-by: One\n> Signed-off-by: Two\n> Signed-off-by: One\n>\n> We clearly should credit both \"One\" and \"Two\", but should \"One\" get\n> credited twice? The current code does so, but mostly because that was\n> the easiest thing to do.\n\nI thought that \"the current documentation\" as of step 4/8 were quite\nclear about double counting ;-).\n\n> It's probably more useful to count each commit\n> at most once. This will become especially important when we allow\n> values from multiple sources in a future patch.\n\nYup. Makes readers want to read further ;-)\n\n"},{"id":"406514","messageId":"xmqqd027ndvv.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927084011.GG2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 7/8] shortlog: parse trailer idents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-27T20:49:56Z","receivedAt":"2020-09-27T20:50:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> +Shortlog will attempt to parse each trailer value as a `name `\n> +identity. If successful, the mailmap is applied and the email is omitted\n> +unless the `--email` option is specified. If the value cannot be parsed\n> +as an identity, it will be taken literally and completely.\n\nMakes sense.\n\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index e6f4faec7c..28133aec68 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -228,6 +228,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n> \tstruct trailer_iterator iter;\n> \tconst char *commit_buffer, *body;\n> \tstruct strset dups = STRSET_INIT;\n> +\tstruct strbuf ident = STRBUF_INIT;\n> \n> \t/*\n> \t * Using format_commit_message(\"%B\") would be simpler here, but\n> @@ -245,12 +246,17 @@ static void insert_records_from_trailers(struct shortlog *log,\n> \t\tif (strcasecmp(iter.key.buf, log->trailer))\n> \t\t\tcontinue;\n> \n> +\t\tstrbuf_reset(&ident);\n> +\t\tif (!parse_ident(log, &ident, value))\n> +\t\t\tvalue = ident.buf;\n\nAnd the implementation is quite straight-forward.\n\n> \t\tif (strset_check_and_add(&dups, value))\n> \t\t\tcontinue;\n> \t\tinsert_one_record(log, value, oneline);\n> \t}\n> \ttrailer_iterator_release(&iter);\n> \n> +\tstrbuf_release(&ident);\n> \tstrset_clear(&dups);\n> \tunuse_commit_buffer(commit, commit_buffer);\n> }\n\nLooking good. Thanks.\n"},{"id":"406516","messageId":"xmqq8scuor59.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927084015.GH2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 8/8] shortlog: allow multiple groups to be specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-27T21:18:10Z","receivedAt":"2020-09-27T21:18:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> -\t\tif (strcasecmp(iter.key.buf, log->trailer))\n> +\t\tif (!string_list_has_string(&log->trailers, iter.key.buf))\n> \t\t\tcontinue;\n...\n> +\tif (!log.groups)\n> +\t\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n> +\tstring_list_sort(&log.trailers);\n\nI initially reacted with \"oh, why sort?\" to this line, before\nrealizing that the list is used as a look-up table, which, if I\nrecall correctly, you said you want to see us move off of in the\nlonger term. \n\nAs we already have the string-set in this series, I am wondering\nwhy we are not using it. It's not like the code at some point\nneeds to iterate over log.trailers in some stable order, right?\n\nI do realize that going to hashmap might be overkill, but once we\nhave an easy-to-use wrapper around it, between one \"table of\nstrings\" API and another \"table of strings\" API, I do not see a\nreason why we want to choose the string_list.\n\nThanks.\n"},{"id":"406519","messageId":"20200928031752.GA3059492@coredump.intra.peff.net","threadId":"54293","inReplyTo":"xmqqlfgvngkw.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 4/8] shortlog: match commit trailers with --group","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-28T03:17:52Z","receivedAt":"2020-09-28T03:17:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 27, 2020 at 12:51:43PM -0700, Junio C Hamano wrote:\n\n> > +The contents of each trailer value are taken literally and completely.\n> > +No mailmap is applied, and the `-e` option has no effect (if the trailer\n> > +contains a username and email, they are both always shown).\n> \n> OK. Some users may find that not quite satisfying, though.\n> \n> But I have a suspicion that the above will be refined in later\n> steps? It would have been nicer to see that mentioned in the\n> proposed log message (e.g. \"this step gives the minimum basics and\n> rough edges like X and Y will be refined with later patches\").\n\nI wondered if this might confuse people reading the series, and almost\ncalled attention to it in the cover letter. Now that you've presumably\nread through and figured it out, is it worth going back and amending the\ncommit message? It's more of a point for reviewers, I think, but perhaps\nsomebody reading the commits later would care.\n\n-Peff\n"},{"id":"406520","messageId":"20200928031921.GB3059492@coredump.intra.peff.net","threadId":"54293","inReplyTo":"xmqqh7rjnf42.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 5/8] shortlog: de-duplicate trailer values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-28T03:19:21Z","receivedAt":"2020-09-28T03:19:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 27, 2020 at 01:23:25PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > The current documentation is vague about what happens with\n> > --group=trailer:signed-off-by when we see a commit with:\n> >\n> > Signed-off-by: One\n> > Signed-off-by: Two\n> > Signed-off-by: One\n> >\n> > We clearly should credit both \"One\" and \"Two\", but should \"One\" get\n> > credited twice? The current code does so, but mostly because that was\n> > the easiest thing to do.\n> \n> I thought that \"the current documentation\" as of step 4/8 were quite\n> clear about double counting ;-).\n\nIt was clear to me that the commit would be counted at least twice: once\nfor the \"one\" bucket and once for the \"two\" bucket, but not that it\nwould count twice for the \"two\" bucket. Or at least, I didn't think of\nthis issue at all back when I wrote the original patch long ago, and\nonly noticed the problem when I revisited it. :)\n\nAnyway, I think the semantics at the end of the series are quite\nsensible.\n\n-Peff\n"},{"id":"406521","messageId":"20200928032515.GC3059492@coredump.intra.peff.net","threadId":"54293","inReplyTo":"xmqq8scuor59.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 8/8] shortlog: allow multiple groups to be specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-28T03:25:15Z","receivedAt":"2020-09-28T03:25:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 27, 2020 at 02:18:10PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > -\t\tif (strcasecmp(iter.key.buf, log->trailer))\n> > +\t\tif (!string_list_has_string(&log->trailers, iter.key.buf))\n> > \t\t\tcontinue;\n> ...\n> > +\tif (!log.groups)\n> > +\t\tlog.groups = SHORTLOG_GROUP_AUTHOR;\n> > +\tstring_list_sort(&log.trailers);\n> \n> I initially reacted with \"oh, why sort?\" to this line, before\n> realizing that the list is used as a look-up table, which, if I\n> recall correctly, you said you want to see us move off of in the\n> longer term.\n> \n> As we already have the string-set in this series, I am wondering\n> why we are not using it. It's not like the code at some point\n> needs to iterate over log.trailers in some stable order, right?\n\nRight, it's purely a lookup, and the new strset or any hashmap would\nwork. However, the trailers list is part of the public shortlog.h[1]\ninterface, so we'd have to move strset to be a public thing, too. I\nthink that may be where we want to go eventually, but I thought I'd\ntrial it as something strictly internal to shortlog.c for now.\n\nI'm also not _too_ bothered by this particular use of string-list as a\nlookup table. It's one of the \"easy\" cases: we build up the list in one\nphase, and then do lookups in the other, so sorting in between is no big\ndeal (though I do think a strset is even easier, as you do not have to\nremember to sort between the phases).\n\n-Peff\n"},{"id":"406546","messageId":"xmqq1rilomx6.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200928031752.GA3059492@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/8] shortlog: match commit trailers with --group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-28T17:01:41Z","receivedAt":"2020-09-28T17:01:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I wondered if this might confuse people reading the series, and almost\n> called attention to it in the cover letter. Now that you've presumably\n> read through and figured it out, is it worth going back and amending the\n> commit message? It's more of a point for reviewers, I think, but perhaps\n> somebody reading the commits later would care.\n\nWhen I read history older than the immediate past, I usuall start at\nthe point 'git blame' found and then go back---going forward is much\nless natural operation. If my \"git blame\" session happens to hit an\nearlier part of this series, perhaps the future-me would get a wrong\nimpression that originally the feature was in a limited form and was\nextended later in a separate series? I dunno.\n\nI did wish the log message said what was going on while reading, but\nthat is a perfectionist in me speaking, and not a complaint that log\nmessages that did not say the future plans in the series were\noffense that deserves a rejection. A good summary in \"What's cooking\"\nreport would become a merge commit log message, and we can teach\nusers to go from 'git blame', find where the entire series was merged\nto the mainline and then read from there---and it would be sufficient\nif the summary of the topic described the endgame well.\n\n\n"},{"id":"413064","messageId":"xmqqbleejhhq.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"20200927084015.GH2465761@coredump.intra.peff.net","subject":"Re: [PATCH v2 8/8] shortlog: allow multiple groups to be specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-28T11:29:53Z","receivedAt":"2020-12-28T11:30:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Now that shortlog supports reading from trailers, it can be useful to\n> combine counts from multiple trailers, or between trailers and authors.\n> This can be done manually by post-processing the output from multiple\n> runs, but it's non-trivial to make sure that each name/commit pair is\n> counted only once.\n>\n> This patch teaches shortlog to accept multiple --group options on the\n> command line, and pull data from all of them. That makes it possible to\n> run:\n>\n> git shortlog -ns --group=author --group=trailer:co-authored-by\n>\n> to get a shortlog that counts authors and co-authors equally.\n\nAs I recently had a chance to revisit an old thread back in v2.3.3\ndays, I recalled that I wished something like this was available,\n\n https://lore.kernel.org/git/xmqqd24g6uf1.fsf@gitster.dls.corp.google.com/\n\nwhere I wished to have a way to count non-author contributions\neasily to list them.\n\nI am not comfortable with the idea of changing the release\nannouncement script immediately before the new release, so I'll do\nv2.30.0 with the old \"authorship only\" script, but I'll play with\nthe new shortlog feature plus ideas from the old thread to see if we\ncan give more credit to non author contributors.\n"},{"id":"416137","messageId":"xmqqy2g48h7x.fsf@gitster.c.googlers.com","threadId":"54293","inReplyTo":"xmqqbleejhhq.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 8/8] shortlog: allow multiple groups to be specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T06:44:34Z","receivedAt":"2021-02-04T06:45:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> Now that shortlog supports reading from trailers, it can be useful to\n>> combine counts from multiple trailers, or between trailers and authors.\n>> This can be done manually by post-processing the output from multiple\n>> runs, but it's non-trivial to make sure that each name/commit pair is\n>> counted only once.\n>>\n>> This patch teaches shortlog to accept multiple --group options on the\n>> command line, and pull data from all of them. That makes it possible to\n>> run:\n>>\n>> git shortlog -ns --group=author --group=trailer:co-authored-by\n>>\n>> to get a shortlog that counts authors and co-authors equally.\n>\n> As I recently had a chance to revisit an old thread back in v2.3.3\n> days, I recalled that I wished something like this was available,\n>\n> https://lore.kernel.org/git/xmqqd24g6uf1.fsf@gitster.dls.corp.google.com/\n>\n> where I wished to have a way to count non-author contributions\n> easily to list them.\n>\n> I am not comfortable with the idea of changing the release\n> announcement script immediately before the new release, so I'll do\n> v2.30.0 with the old \"authorship only\" script, but I'll play with\n> the new shortlog feature plus ideas from the old thread to see if we\n> can give more credit to non author contributors.\n\nBut now it is a good time to do so, as it is not immediately before\na new release.\n\nHere is what I am planning to use for the next release due in mid\nMarch. We list those who had contribution in the part of the\nhistory before the last release and in the part of the history since\nthe last release, and run \"comm\" on these two sets to derive \"new\nfolks\" vs \"continued supporters\". The list of contributors were\nonly counting the commit authors in the previous announcement, but\nwith the \"shortlog --group\" feature, we can give folks credit for\nall kinds of *-by roles.\n\ndiff --git a/Announce b/Announce\nindex 491516b..d6ec20c 100755\n--- a/Announce\n+++ b/Announce\n@@ -44,8 +44,22 @@ esac\n \n vername=$(echo \"$vername\" | tr \"-\" \".\")\n \n-git log --use-mailmap --format='%aN,' \"$previous\" | sort -u >\"$tmpbase.prev\"\n-git log --use-mailmap --format='%aN,' \"$previous..$commit\" | sort -u >\"$tmpbase.this\"\n+people () {\n+\tgit shortlog -s --no-merges \\\n+\t\t--group=author \\\n+\t\t--group=trailer:co-authored-by \\\n+\t\t--group=trailer:reviewed-by \\\n+\t\t--group=trailer:mentored-by \\\n+\t\t--group=trailer:helped-by \\\n+\t\t--group=trailer:reported-by \\\n+\t\t\"$@\" |\n+\tsed -e 's/^[ \t0-9]*[ \t]//' -e 's/$/,/' |\n+\tsort -u\n+}\n+\n+people \"$previous\" >\"$tmpbase.prev\"\n+people \"$previous..$commit\" >\"$tmpbase.this\"\n+\n comm -12 \"$tmpbase.prev\" \"$tmpbase.this\" >\"$tmpbase.old\"\n comm -13 \"$tmpbase.prev\" \"$tmpbase.this\" >\"$tmpbase.new\"\n \n\n"}]}