{"thread":{"id":"59845","subject":"[PATCH 0/2] shortlog: introduce --email-only, --group-filter options","startedAt":"2023-06-07T23:02:14Z","lastAt":"2023-06-12T21:25:28Z","messageCount":8,"participants":["Taylor Blau","Johannes Sixt","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"478160","messageId":"cover.1686178917.git.me@ttaylorr.com","threadId":"59845","inReplyTo":null,"subject":"[PATCH 0/2] shortlog: introduce --email-only, --group-filter options","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-07T23:02:06Z","receivedAt":"2023-06-07T23:02:14Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"These couple of patches introduce a pair of new options to `git\nshortlog`. They are:\n\n  - `--group-filter`, which restricts the output to group(s) of interest\n\n  - `--email-only`, which allows callers to splice emails out of groups,\n    which is useful when grouping based on trailers that usually contain\n    identities (e.g. \"Co-authored-by\"), where the search is\n    name-insensitive and only cares about a strict match on the email.\n\nLet me know what you think, and thanks in advance for your review.\n\nTaylor Blau (2):\n  shortlog: introduce `--group-filter` to restrict output\n  shortlog: introduce `--email-only` to only show emails\n\n Documentation/git-shortlog.txt | 10 +++++\n builtin/shortlog.c             | 53 +++++++++++++++++-----\n shortlog.h                     |  3 ++\n t/t4201-shortlog.sh            | 82 ++++++++++++++++++++++++++++++++++\n 4 files changed, 138 insertions(+), 10 deletions(-)\n\n-- \n2.41.0.1.gf123b68cb8\n"},{"id":"478161","messageId":"5cec04b65d350ca8b482ca14260ef118341e4039.1686178917.git.me@ttaylorr.com","threadId":"59845","inReplyTo":"cover.1686178917.git.me@ttaylorr.com","subject":"[PATCH 1/2] shortlog: introduce `--group-filter` to restrict output","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-07T23:02:11Z","receivedAt":"2023-06-07T23:02:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Sometimes it is useful to ask, \"how many commits have I authored or been\nmentioned in via the Co-authored-by trailer\"? `git shortlog` is a\nreasonable tool for this, and the answer can easily be obtained by\nrunning:\n\n    $ git shortlog -ns --group=author --group=trailer:Co-authored-by\n\nand reading off the corresponding value from its output. But what if you\nwant to know which commits contribute to the above count? You can drop\nthe `-s` option and parse through the results, but you'll have to skip\npast everything you don't want (and stop reading after matching\neverything you do want).\n\nThat is a script-able task, but it is cumbersome, and potentially very\nslow, if there is a large amount of output that does not match the given\nquery.\n\nInstead, this patch introduces `--group-filter` in order to restrict the\noutput of `git shortlog` to only matching group(s). Items match if they\nare in a group which is strictly equal to one of the specified filters.\n\nThis means that you could easily view the hashes of all commits you\neither wrote or co-authored with something like:\n\n    $ git shortlog -n --group=author --group=trailer:Co-authored-by \\\n        --group-filter=\"$(git config user.name)\"\n\nWhen filtering just by trailers, it is tempting to want to introduce a\nnew grep mode for matching a given trailer, like `--author=<pattern>`\nfor matching the author header. But this would not be suitable for the\nabove, since we want commits which match either the author or the\nCo-authored-by trailer, not ones which match both.\n\nAn alternative approach might be to implement trailer filtering as\nabove in revision.c, and show commits matching either the `--author`\nvalue or some hypothetical `--trailer` filter. That would give shortlog\nfewer commits, which may improve its performance. But it would restrict\nthe interpretation of these options to be an OR (i.e. show commits\nmatching either the `--author` or `--trailer` field).\n\nIn fact, this is already not possible to do, since the `--author` and\n`--committer` options are documented as:\n\n> Limit the commits output to ones with author/committer header lines\n> that match the specified pattern\n\nSo introducing another option which changes the behavior of existing\nones is a non-starter.\n\nInstead, `git shortlog` will process more commits than necessary. But\nthis is a marginal cost, since implementing the hypothetical revision\noptions from above as an OR would mean that revision.c has to process\nevery commit anyway.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-shortlog.txt |  5 ++++\n builtin/shortlog.c             | 20 ++++++++++++-\n shortlog.h                     |  2 ++\n t/t4201-shortlog.sh            | 54 ++++++++++++++++++++++++++++++++++\n 4 files changed, 80 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 7d0277d033..dab6d09648 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -97,6 +97,11 @@ counts both authors and co-authors.\n If width is `0` (zero) then indent the lines of the output without wrapping\n them.\n \n+--group-filter=<group>::\n+\tOnly show output from the given group. If given more than once,\n+\tshow output from any of the previously specified groups. May be\n+\tcleared with `--no-group-filter`.\n+\n <revision-range>::\n \tShow only commits in the specified revision range.  When no\n \t<revision-range> is specified, it defaults to `HEAD` (i.e. the\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 46f4e0832a..679db22c57 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -365,6 +365,7 @@ void shortlog_init(struct shortlog *log)\n \tlog->trailers.strdup_strings = 1;\n \tlog->trailers.cmp = strcasecmp;\n \tlog->format.strdup_strings = 1;\n+\tlog->group_filter.strdup_strings = 1;\n }\n \n void shortlog_finish_setup(struct shortlog *log)\n@@ -377,6 +378,7 @@ void shortlog_finish_setup(struct shortlog *log)\n \t\t\t\t   log->email ? \"%cN <%cE>\" : \"%cN\");\n \n \tstring_list_sort(&log->trailers);\n+\tstring_list_sort(&log->group_filter);\n }\n \n int cmd_shortlog(int argc, const char **argv, const char *prefix)\n@@ -400,6 +402,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\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_STRING_LIST(0, \"group-filter\", &log.group_filter,\n+\t\t\t\tN_(\"group\"), N_(\"only show matching groups\")),\n \t\tOPT_END(),\n \t};\n \n@@ -476,6 +480,13 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,\n \tstrbuf_addch(sb, '\\n');\n }\n \n+static int want_shortlog_group(struct shortlog *log, const char *group)\n+{\n+\tif (!log->group_filter.nr)\n+\t\treturn 1;\n+\treturn string_list_has_string(&log->group_filter, group);\n+}\n+\n void shortlog_output(struct shortlog *log)\n {\n \tsize_t i, j;\n@@ -486,6 +497,9 @@ void shortlog_output(struct shortlog *log)\n \t\t      log->summary ? compare_by_counter : compare_by_list);\n \tfor (i = 0; i < log->list.nr; i++) {\n \t\tconst struct string_list_item *item = &log->list.items[i];\n+\t\tif (!want_shortlog_group(log, item->string))\n+\t\t\tgoto next;\n+\n \t\tif (log->summary) {\n \t\t\tfprintf(log->file, \"%6d\\t%s\\n\",\n \t\t\t\t(int)UTIL_TO_INT(item), item->string);\n@@ -505,11 +519,15 @@ void shortlog_output(struct shortlog *log)\n \t\t\t\t\tfprintf(log->file, \"      %s\\n\", msg);\n \t\t\t}\n \t\t\tputc('\\n', log->file);\n+\t\t}\n+\n+next:\n+\t\tif (!log->summary) {\n+\t\t\tstruct string_list *onelines = item->util;\n \t\t\tonelines->strdup_strings = 1;\n \t\t\tstring_list_clear(onelines, 0);\n \t\t\tfree(onelines);\n \t\t}\n-\n \t\tlog->list.items[i].util = NULL;\n \t}\n \ndiff --git a/shortlog.h b/shortlog.h\nindex 28d04f951a..8ebee0e2d6 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -18,6 +18,8 @@ struct shortlog {\n \tint abbrev;\n \tstruct date_mode date_mode;\n \n+\tstruct string_list group_filter;\n+\n \tenum {\n \t\tSHORTLOG_GROUP_AUTHOR = (1 << 0),\n \t\tSHORTLOG_GROUP_COMMITTER = (1 << 1),\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 8e4effebdb..0695c42ca8 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -346,6 +346,60 @@ test_expect_success 'shortlog can match multiple groups' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--group-filter shows only matching groups (single groups)' '\n+\tcat >expect <<-\\EOF &&\n+\t     1\tA U Thor\n+\tEOF\n+\tgit shortlog -ns \\\n+\t\t--group=trailer:another-trailer \\\n+\t\t--group-filter=\"A U Thor\" \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--group-filter shows only matching groups (multiple groups)' '\n+\tcat >expect <<-\\EOF &&\n+\t     2\tA U Thor\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--group-filter=\"A U Thor\" \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--group-filter can be specified more than once' '\n+\tcat >expect <<-\\EOF &&\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--group-filter=\"User A\" \\\n+\t\t--group-filter=\"User B\" \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-group-filter reset group filters' '\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--group-filter=\"A U Thor\" --no-group-filter \\\n+\t\t-2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'shortlog can match multiple format groups' '\n \tGIT_COMMITTER_NAME=\"$GIT_AUTHOR_NAME\" \\\n \t\tgit commit --allow-empty -m \"identical names\" &&\n-- \n2.41.0.1.gf123b68cb8\n\n"},{"id":"478162","messageId":"44179d28fa7676965a28734e20584d54b44e051b.1686178917.git.me@ttaylorr.com","threadId":"59845","inReplyTo":"cover.1686178917.git.me@ttaylorr.com","subject":"[PATCH 2/2] shortlog: introduce `--email-only` to only show emails","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-07T23:02:16Z","receivedAt":"2023-06-07T23:02:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When a shortlog caller wants to group output by, say, author email, they\ncan easily express this with:\n\n    $ git shortlog --group=format:%ae\n\nand restrict output to specific email(s) with the new `--group-filter`\noption introduced by the previous commit.\n\nBut they are not able to apply the same treatment to identities that\nappear in trailers. Doing:\n\n    $ git shortlog -e --group=format:%ae --group=trailer:Co-authored-by\n\nwill produce funky results, interspersing proper emails with full \"Name\n<email>\" identities from the Co-authored-by trailer (or anything else\nthat might appear there), like:\n\n    461  me@ttaylorr.com\n     11  Taylor Blau <me@ttaylorr.com>\n\nSo if the caller wants to restrict output to a set of matching email\naddresses (say, \"me@ttaylorr.com\"), they cannot do it with a\n`--group-filter`, since it would discard the group \"Taylor Blau\n<me@ttaylorr.com>\".\n\nIntroduce a new `--email-only` option, which extracts the email\ncomponent of an identity from all shortlog groups, including trailers.\nIt behaves similarly to the `-e` option, but replaces its output with\njust the email component, instead of adding it on to the end.\n\nNow, `shortlog` callers can perform:\n\n    $ git shortlog -s --group=author --group=trailer:Co-authored-by \\\n        --email-only --group-filter=\"<me@ttaylorr.com>\"\n       472  <me@ttaylorr.com>\n\nto obtain the output they want.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-shortlog.txt |  5 +++++\n builtin/shortlog.c             | 33 ++++++++++++++++++++++++---------\n shortlog.h                     |  1 +\n t/t4201-shortlog.sh            | 28 ++++++++++++++++++++++++++++\n 4 files changed, 58 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex dab6d09648..160c11aead 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -39,6 +39,11 @@ OPTIONS\n --email::\n \tShow the email address of each author.\n \n+--email-only::\n+\tShow only the email address of each author, or committer. If\n+\tusing a different kind of group (e.g. trailers, custom format,\n+\tetc.), only values which contain name/email-pairs will be shown.\n+\n --format[=<format>]::\n \tInstead of the commit subject, use some other information to\n \tdescribe each commit.  '<format>' can be any string accepted\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 679db22c57..bbe01a376f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -108,9 +108,13 @@ static int parse_ident(struct shortlog *log,\n \tmaillen = ident.mail_end - ident.mail_begin;\n \n \tmap_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n-\tstrbuf_add(out, namebuf, namelen);\n-\tif (log->email)\n-\t\tstrbuf_addf(out, \" <%.*s>\", (int)maillen, mailbuf);\n+\tif (log->email_only) {\n+\t\tstrbuf_addf(out, \"<%.*s>\", (int)maillen, mailbuf);\n+\t} else {\n+\t\tstrbuf_add(out, namebuf, namelen);\n+\t\tif (log->email)\n+\t\t\tstrbuf_addf(out, \" <%.*s>\", (int)maillen, mailbuf);\n+\t}\n \n \treturn 0;\n }\n@@ -198,6 +202,8 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\tstrbuf_reset(&ident);\n \t\tif (!parse_ident(log, &ident, value))\n \t\t\tvalue = ident.buf;\n+\t\telse if (log->email_only)\n+\t\t\tcontinue;\n \n \t\tif (!strset_add(dups, value))\n \t\t\tcontinue;\n@@ -370,12 +376,19 @@ void shortlog_init(struct shortlog *log)\n \n void shortlog_finish_setup(struct shortlog *log)\n {\n-\tif (log->groups & SHORTLOG_GROUP_AUTHOR)\n-\t\tstring_list_append(&log->format,\n-\t\t\t\t   log->email ? \"%aN <%aE>\" : \"%aN\");\n-\tif (log->groups & SHORTLOG_GROUP_COMMITTER)\n-\t\tstring_list_append(&log->format,\n-\t\t\t\t   log->email ? \"%cN <%cE>\" : \"%cN\");\n+\tif (log->email_only) {\n+\t\tif (log->groups & SHORTLOG_GROUP_AUTHOR)\n+\t\t\tstring_list_append(&log->format, \"<%aE>\");\n+\t\tif (log->groups & SHORTLOG_GROUP_COMMITTER)\n+\t\t\tstring_list_append(&log->format, \"<%cE>\");\n+\t} else {\n+\t\tif (log->groups & SHORTLOG_GROUP_AUTHOR)\n+\t\t\tstring_list_append(&log->format,\n+\t\t\t\t\t   log->email ? \"%aN <%aE>\" : \"%aN\");\n+\t\tif (log->groups & SHORTLOG_GROUP_COMMITTER)\n+\t\t\tstring_list_append(&log->format,\n+\t\t\t\t\t   log->email ? \"%cN <%cE>\" : \"%cN\");\n+\t}\n \n \tstring_list_sort(&log->trailers);\n \tstring_list_sort(&log->group_filter);\n@@ -397,6 +410,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"suppress commit descriptions, only provides commit count\")),\n \t\tOPT_BOOL('e', \"email\", &log.email,\n \t\t\t N_(\"show the email address of each author\")),\n+\t\tOPT_BOOL(0, \"email-only\", &log.email_only,\n+\t\t\t N_(\"only show the email address of each author\")),\n \t\tOPT_CALLBACK_F('w', NULL, &log, N_(\"<w>[,<i1>[,<i2>]]\"),\n \t\t\tN_(\"linewrap output\"), PARSE_OPT_OPTARG,\n \t\t\t&parse_wrap_args),\ndiff --git a/shortlog.h b/shortlog.h\nindex 8ebee0e2d6..3fb28639c3 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -30,6 +30,7 @@ struct shortlog {\n \tstruct string_list format;\n \n \tint email;\n+\tint email_only;\n \tstruct string_list mailmap;\n \tFILE *file;\n };\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 0695c42ca8..d747a402ff 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -400,6 +400,34 @@ test_expect_success '--no-group-filter reset group filters' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--email-only shows emails from author' '\n+\tcat >expect <<-\\EOF &&\n+\t     2\t<author@example.com>\n+\tEOF\n+\tgit shortlog -ns --group=author --email-only -2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--email-only shows emails from committer' '\n+\tcat >expect <<-\\EOF &&\n+\t     2\t<committer@example.com>\n+\tEOF\n+\tgit shortlog -ns --group=committer --email-only -2 HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--email-only shows emails from trailers with idents' '\n+\tcat >expect <<-\\EOF &&\n+\t     1\t<a@example.com>\n+\t     1\t<b@example.com>\n+\tEOF\n+\t# at this point, HEAD~3 has a trailer \"Repeated-trailer: Foo\",\n+\t# which is not shown here since it cannot be parsed as an ident\n+\tgit shortlog -ns --group=trailer:some-trailer --email-only -3 \\\n+\t\tHEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'shortlog can match multiple format groups' '\n \tGIT_COMMITTER_NAME=\"$GIT_AUTHOR_NAME\" \\\n \t\tgit commit --allow-empty -m \"identical names\" &&\n-- \n2.41.0.1.gf123b68cb8\n"},{"id":"478164","messageId":"67c66cd7-fb6c-3fa0-82c8-0e778f377f96@kdbg.org","threadId":"59845","inReplyTo":"44179d28fa7676965a28734e20584d54b44e051b.1686178917.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] shortlog: introduce `--email-only` to only show emails","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2023-06-08T07:35:14Z","receivedAt":"2023-06-08T08:21:24Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.06.23 um 01:02 schrieb Taylor Blau:\n> When a shortlog caller wants to group output by, say, author email, they\n> can easily express this with:\n> \n>     $ git shortlog --group=format:%ae\n> \n> and restrict output to specific email(s) with the new `--group-filter`\n> option introduced by the previous commit.\n> \n> But they are not able to apply the same treatment to identities that\n> appear in trailers. Doing:\n> \n>     $ git shortlog -e --group=format:%ae --group=trailer:Co-authored-by\n> \n> will produce funky results, interspersing proper emails with full \"Name\n> <email>\" identities from the Co-authored-by trailer (or anything else\n> that might appear there), like:\n> \n>     461  me@ttaylorr.com\n>      11  Taylor Blau <me@ttaylorr.com>\n> \n> So if the caller wants to restrict output to a set of matching email\n> addresses (say, \"me@ttaylorr.com\"), they cannot do it with a\n> `--group-filter`, since it would discard the group \"Taylor Blau\n> <me@ttaylorr.com>\".\n> \n> Introduce a new `--email-only` option, which extracts the email\n> component of an identity from all shortlog groups, including trailers.\n> It behaves similarly to the `-e` option, but replaces its output with\n> just the email component, instead of adding it on to the end.\n> \n> Now, `shortlog` callers can perform:\n> \n>     $ git shortlog -s --group=author --group=trailer:Co-authored-by \\\n>         --email-only --group-filter=\"<me@ttaylorr.com>\"\n>        472  <me@ttaylorr.com>\n> \n> to obtain the output they want.\n\nA note from the peanut gallery: \"--email-only\" sounds like an option\nthat affects the output of the command. But it does not (IIUC), there is\nno hint that it affects grouping and filtering. It is named too\ngenerically, IMHO. Can we not have the desired effect by specifying some\ntoken to one of the --group* options?\n\n-- Hannes\n\n"},{"id":"478165","messageId":"3778f07f-6e6f-5a39-631d-1266d61b9715@github.com","threadId":"59845","inReplyTo":"5cec04b65d350ca8b482ca14260ef118341e4039.1686178917.git.me@ttaylorr.com","subject":"Re: [PATCH 1/2] shortlog: introduce `--group-filter` to restrict output","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-06-08T14:34:02Z","receivedAt":"2023-06-08T14:34:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/7/2023 7:02 PM, Taylor Blau wrote:\n\n> This means that you could easily view the hashes of all commits you\n> either wrote or co-authored with something like:\n> \n>     $ git shortlog -n --group=author --group=trailer:Co-authored-by \\\n>         --group-filter=\"$(git config user.name)\"\n> \n> When filtering just by trailers, it is tempting to want to introduce a\n> new grep mode for matching a given trailer, like `--author=<pattern>`\n> for matching the author header. But this would not be suitable for the\n> above, since we want commits which match either the author or the\n> Co-authored-by trailer, not ones which match both.\n\nOne thing that is not immediately obvious from reading the patch, but\nbecomes clearer in patch 2, is that your --group-filter is an exact\nstring match. This differs from the --author filter in 'git log' and\nsimilar, which is actually a case-insensitive substring match.\n\n> +static int want_shortlog_group(struct shortlog *log, const char *group)\n> +{\n> +\tif (!log->group_filter.nr)\n> +\t\treturn 1;\n> +\treturn string_list_has_string(&log->group_filter, group);\n> +}\n> +\n\nThis is the critical piece of code for this issue. Replacing it with\n\nstatic int want_shortlog_group(struct shortlog *log, const char *group)\n{\n\tstruct string_list_item *item;\n\tif (!log->group_filter.nr)\n\t\treturn 1;\n\n\tfor_each_string_list_item(item, &log->group_filter) {\n\t\tif (strcasestr(group, item->string))\n\t\t\treturn 1;\n\t}\n\n\treturn 0;\n}\n\nResults in the case-insensitive substring search that I would expect\nfrom this parameter.\n\nThis would also solve the problem from Patch 2 where we want to search\nby email address. Using '-e --group-filter=\"my@email.com\"' works, though\nit will catch users with 'tammy@email.com' emails, as well.\n\nSomething to consider at a high level before committing to this CLI.\n\nThanks,\n-Stolee\n"},{"id":"478166","messageId":"ZIIAUAEkWNQhc5zi@nand.local","threadId":"59845","inReplyTo":"3778f07f-6e6f-5a39-631d-1266d61b9715@github.com","subject":"Re: [PATCH 1/2] shortlog: introduce `--group-filter` to restrict output","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-08T16:22:40Z","receivedAt":"2023-06-08T16:22:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jun 08, 2023 at 10:34:02AM -0400, Derrick Stolee wrote:\n> On 6/7/2023 7:02 PM, Taylor Blau wrote:\n>\n> > This means that you could easily view the hashes of all commits you\n> > either wrote or co-authored with something like:\n> >\n> >     $ git shortlog -n --group=author --group=trailer:Co-authored-by \\\n> >         --group-filter=\"$(git config user.name)\"\n> >\n> > When filtering just by trailers, it is tempting to want to introduce a\n> > new grep mode for matching a given trailer, like `--author=<pattern>`\n> > for matching the author header. But this would not be suitable for the\n> > above, since we want commits which match either the author or the\n> > Co-authored-by trailer, not ones which match both.\n>\n> One thing that is not immediately obvious from reading the patch, but\n> becomes clearer in patch 2, is that your --group-filter is an exact\n> string match. This differs from the --author filter in 'git log' and\n> similar, which is actually a case-insensitive substring match.\n\nYeah, funnily enough I originally thought about adding a `--trailer`\nflag to the revision machinery, but realized that it was a non-starter\nsince `--author` is documented as only showing commits matching that\nauthor.\n\nSo that doesn't solve for the case of finding a commit which has the\nidentity you want buried in some trailer, but is written by a different\nauthor. shortlog needs to see all commits along the traversal, so I went\nwith the filtering approach instead.\n\nThanks for the pointer on beefing up the documentation to indicate that\nthis is an exact search.\n\n> This is the critical piece of code for this issue. Replacing it with\n>\n> static int want_shortlog_group(struct shortlog *log, const char *group)\n> {\n> \tstruct string_list_item *item;\n> \tif (!log->group_filter.nr)\n> \t\treturn 1;\n>\n> \tfor_each_string_list_item(item, &log->group_filter) {\n> \t\tif (strcasestr(group, item->string))\n> \t\t\treturn 1;\n> \t}\n>\n> \treturn 0;\n> }\n>\n> Results in the case-insensitive substring search that I would expect\n> from this parameter.\n>\n> This would also solve the problem from Patch 2 where we want to search\n> by email address. Using '-e --group-filter=\"my@email.com\"' works, though\n> it will catch users with 'tammy@email.com' emails, as well.\n\nYeah, thanks for raising it. I wonder if there are other semantics that\ndon't incorrectly return substring matches. You could conceivably extend\nthe --group-filter argument to take an extended regular expression, and\nthen just match on whatever's inside the last \"<>\".\n\nThat approach was one that I considered, but feels a little heavyweight\nfor what we're trying to do here. One potential approach is to keep\nthings as-is, and then consider extending `--group-filter` to mean\n\"extended regular expression\" in the future. Although I'm not sure that\nwould work, either, since fixed-string matches would suddenly match\nanywhere in the string, breaking backwards compatibility.\n\nYou could add another option that specifies the behavior of\n`--group-filter`, or add some extra bit of information there like\n`--group-filter=egrep:<pattern>` or something.\n\nAt least there's no shortage of options ;-).\n\nThanks,\nTaylor\n"},{"id":"478167","messageId":"ZIIAtO3kSaKkPqu8@nand.local","threadId":"59845","inReplyTo":"67c66cd7-fb6c-3fa0-82c8-0e778f377f96@kdbg.org","subject":"Re: [PATCH 2/2] shortlog: introduce `--email-only` to only show emails","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-08T16:24:20Z","receivedAt":"2023-06-08T16:24:52Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jun 08, 2023 at 09:35:14AM +0200, Johannes Sixt wrote:\n> A note from the peanut gallery: \"--email-only\" sounds like an option\n> that affects the output of the command. But it does not (IIUC), there is\n> no hint that it affects grouping and filtering. It is named too\n> generically, IMHO. Can we not have the desired effect by specifying some\n> token to one of the --group* options?\n\nI had thought about the possibility of something like this since we (a)\nalready have lots of `%(trailer)`-related pretty formatting options, and\n(b) support grouping by pretty formatted-commits with the\n`--group=format:<format>` option I introduced via c112d8d9c2 (Merge\nbranch 'tb/shortlog-group', 2022-10-30).\n\nThanks,\nTaylor\n"},{"id":"478307","messageId":"xmqqlego8jf6.fsf@gitster.g","threadId":"59845","inReplyTo":"67c66cd7-fb6c-3fa0-82c8-0e778f377f96@kdbg.org","subject":"Re: [PATCH 2/2] shortlog: introduce `--email-only` to only show emails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-12T21:22:53Z","receivedAt":"2023-06-12T21:25:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> A note from the peanut gallery: \"--email-only\" sounds like an option\n> that affects the output of the command.\n\nGood observation.  I had exactly the same reaction.\n"}]}