{"thread":{"id":"60251","subject":"[PATCH 0/2] Add mailmap support to ref-filter","startedAt":"2023-09-20T19:17:40Z","lastAt":"2023-09-25T17:51:17Z","messageCount":11,"participants":["Kousik Sanagavarapu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"482082","messageId":"20230920191654.6133-1-five231003@gmail.com","threadId":"60251","inReplyTo":null,"subject":"[PATCH 0/2] Add mailmap support to ref-filter","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-20T19:05:40Z","receivedAt":"2023-09-20T19:17:40Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Add mailmap support to ref-filter, making ref-filter and pretty closer, which\nis a part of the effort made to unify both ref-filter and pretty.\n\nPATCH 1/2 - Introduces the \"test_bad_atom()\" function which checks for if the\n\t    given error message (either due to \"err_bad_arg()\" or any other err)\n\t    is correct.\n\nPATCH 2/2 - The actual mailmap support.\n\nKousik Sanagavarapu (2):\n  t/t6300: introduce test_bad_atom()\n  ref-filter: add mailmap support\n\n Documentation/git-for-each-ref.txt |   6 +-\n ref-filter.c                       | 152 ++++++++++++++++++++++-------\n t/t6300-for-each-ref.sh            | 103 +++++++++++++++++++\n 3 files changed, 225 insertions(+), 36 deletions(-)\n\n-- \n2.42.0.160.g6905eb16ce.dirty\n\n"},{"id":"482083","messageId":"20230920191654.6133-2-five231003@gmail.com","threadId":"60251","inReplyTo":"20230920191654.6133-1-five231003@gmail.com","subject":"[PATCH 1/2] t/t6300: introduce test_bad_atom()","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-20T19:05:41Z","receivedAt":"2023-09-20T19:17:45Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Introduce a new function \"test_bad_atom()\", which is similar to\n\"test_atom()\" but should be used to check whether the correct error\nmessage is shown on stderr.\n\nLike \"test_atom()\", the new function takes three arguments. The three\narguments specify the ref, the format and the expected error message\nrespectively, with an optional fourth argument for tweaking\n\"test_expect_*\" (which is by default \"success\").\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n---\n t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 7b943fd34c..15b4622f57 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers\n \ttest_must_fail git for-each-ref --format=\"%(objectname:short=foo)\"\n '\n \n+test_bad_atom() {\n+\tcase \"$1\" in\n+\thead) ref=refs/heads/main ;;\n+\t tag) ref=refs/tags/testtag ;;\n+\t sym) ref=refs/heads/sym ;;\n+\t   *) ref=$1 ;;\n+\tesac\n+\tprintf '%s\\n' \"$3\">expect\n+\ttest_expect_${4:-success} $PREREQ \"err basic atom: $1 $2\" \"\n+\t\ttest_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+test_bad_atom head 'authoremail:foo' \\\n+\t'fatal: unrecognized %(authoremail) argument: foo'\n+\n+test_bad_atom tag 'taggeremail:localpart trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument:  trim'\n+\n test_date () {\n \tf=$1 &&\n \tcommitter_date=$2 &&\n-- \n2.42.0.160.g6905eb16ce.dirty\n\n"},{"id":"482084","messageId":"20230920191654.6133-3-five231003@gmail.com","threadId":"60251","inReplyTo":"20230920191654.6133-1-five231003@gmail.com","subject":"[PATCH 2/2] ref-filter: add mailmap support","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-20T19:05:42Z","receivedAt":"2023-09-20T19:17:46Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Add mailmap support to ref-filter formats which are similar in\npretty. This support is such that the following pretty placeholders are\nequivalent to the new ref-filter atoms:\n\n\t%aN = authorname:mailmap\n\t%cN = committername:mailmap\n\n\t%aE = authoremail:mailmap\n\t%aL = authoremail:mailmap,localpart\n\t%cE = committeremail:mailmap\n\t%cL = committeremail:mailmap,localpart\n\nAdditionally, mailmap can also be used with \":trim\" option for email by\ndoing something like \"authoremail:mailmap,trim\".\n\nThe above also applies for the \"tagger\" atom, that is,\n\"taggername:mailmap\", \"taggeremail:mailmap\", \"taggeremail:mailmap,trim\"\nand \"taggername:mailmap,localpart\".\n\nThe functionality of \":trim\" and \":localpart\" remains the same. That is,\n\":trim\" gives the email, but without the angle brackets and \":localpart\"\ngives the part of the email before the '@' character (if such a\ncharacter is not found then we directly grab everything between the\nangle brackets).\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n---\n Documentation/git-for-each-ref.txt |   6 +-\n ref-filter.c                       | 152 ++++++++++++++++++++++-------\n t/t6300-for-each-ref.sh            |  83 ++++++++++++++++\n 3 files changed, 205 insertions(+), 36 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 11b2bc3121..e86d5700dd 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,\n and `date` to extract the named component.  For email fields (`authoremail`,\n `committeremail` and `taggeremail`), `:trim` can be appended to get the email\n without angle brackets, and `:localpart` to get the part before the `@` symbol\n-out of the trimmed email.\n+out of the trimmed email. In addition to these, the `:mailmap` option and the\n+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does\n+not matter) to get values of the name and email according to the .mailmap file\n+or according to the file set in the mailmap.file or mailmap.blob configuration\n+variable (see linkgit:gitmailmap[5]).\n \n The raw data in an object is `raw`.\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex fae9f4b8ed..e4d3510e28 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -13,6 +13,8 @@\n #include \"oid-array.h\"\n #include \"repository.h\"\n #include \"commit.h\"\n+#include \"mailmap.h\"\n+#include \"ident.h\"\n #include \"remote.h\"\n #include \"color.h\"\n #include \"tag.h\"\n@@ -215,8 +217,16 @@ static struct used_atom {\n \t\tstruct {\n \t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n \t\t} objectsize;\n-\t\tstruct email_option {\n-\t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n+\t\tstruct {\n+\t\t\tenum { N_RAW, N_MAILMAP } option;\n+\t\t} name_option;\n+\t\tstruct {\n+\t\t\tenum {\n+\t\t\t\tEO_RAW = 0,\n+\t\t\t\tEO_TRIM = 1<<0,\n+\t\t\t\tEO_LOCALPART = 1<<1,\n+\t\t\t\tEO_MAILMAP = 1<<2,\n+\t\t\t} option;\n \t\t} email_option;\n \t\tstruct {\n \t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY,\n@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,\n \treturn 0;\n }\n \n-static int person_email_atom_parser(struct ref_format *format UNUSED,\n-\t\t\t\t    struct used_atom *atom,\n-\t\t\t\t    const char *arg, struct strbuf *err)\n+static int person_name_atom_parser(struct ref_format *format UNUSED,\n+\t\t\t\t   struct used_atom *atom,\n+\t\t\t\t   const char *arg, struct strbuf *err)\n {\n \tif (!arg)\n-\t\tatom->u.email_option.option = EO_RAW;\n-\telse if (!strcmp(arg, \"trim\"))\n-\t\tatom->u.email_option.option = EO_TRIM;\n-\telse if (!strcmp(arg, \"localpart\"))\n-\t\tatom->u.email_option.option = EO_LOCALPART;\n+\t\tatom->u.name_option.option = N_RAW;\n+\telse if (!strcmp(arg, \"mailmap\"))\n+\t\tatom->u.name_option.option = N_MAILMAP;\n \telse\n \t\treturn err_bad_arg(err, atom->name, arg);\n \treturn 0;\n }\n \n+static int email_atom_option_parser(struct used_atom *atom,\n+\t\t\t\t    const char **arg, struct strbuf *err)\n+{\n+\tif (!*arg)\n+\t\treturn EO_RAW;\n+\tif (skip_prefix(*arg, \"trim\", arg))\n+\t\treturn EO_TRIM;\n+\tif (skip_prefix(*arg, \"localpart\", arg))\n+\t\treturn EO_LOCALPART;\n+\tif (skip_prefix(*arg, \"mailmap\", arg))\n+\t\treturn EO_MAILMAP;\n+\treturn -1;\n+}\n+\n+static int person_email_atom_parser(struct ref_format *format UNUSED,\n+\t\t\t\t    struct used_atom *atom,\n+\t\t\t\t    const char *arg, struct strbuf *err)\n+{\n+\tfor (;;) {\n+\t\tint opt = email_atom_option_parser(atom, &arg, err);\n+\t\tconst char *bad_arg = arg;\n+\n+\t\tif (opt < 0)\n+\t\t\treturn err_bad_arg(err, atom->name, bad_arg);\n+\t\tatom->u.email_option.option |= opt;\n+\n+\t\tif (!arg || !*arg)\n+\t\t\tbreak;\n+\t\tif (*arg == ',')\n+\t\t\targ++;\n+\t\telse\n+\t\t\treturn err_bad_arg(err, atom->name, bad_arg);\n+\t}\n+\treturn 0;\n+}\n+\n static int refname_atom_parser(struct ref_format *format UNUSED,\n \t\t\t       struct used_atom *atom,\n \t\t\t       const char *arg, struct strbuf *err)\n@@ -877,15 +921,15 @@ static struct {\n \t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n \t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n \t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n-\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n+\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n-\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n+\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n-\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n+\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)\n \treturn xstrdup(\"\");\n }\n \n+static const char *find_end_of_email(const char *email, int opt)\n+{\n+\tconst char *eoemail;\n+\n+\tif (opt & EO_LOCALPART) {\n+\t\teoemail = strchr(email, '@');\n+\t\tif (eoemail)\n+\t\t\treturn eoemail;\n+\t\treturn strchr(email, '>');\n+\t}\n+\n+\tif (opt & EO_TRIM)\n+\t\treturn strchr(email, '>');\n+\n+\t/*\n+\t * The option here is either the raw email option or the raw\n+\t * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,\n+\t * we directly grab the whole email including the closing\n+\t * angle brackets.\n+\t *\n+\t * If EO_MAILMAP was set with any other option (that is either\n+\t * EO_TRIM or EO_LOCALPART), we already grab the end of email\n+\t * above.\n+\t */\n+\teoemail = strchr(email, '>');\n+\tif (eoemail)\n+\t\teoemail++;\n+\treturn eoemail;\n+}\n+\n static const char *copy_email(const char *buf, struct used_atom *atom)\n {\n \tconst char *email = strchr(buf, '<');\n \tconst char *eoemail;\n+\tint opt = atom->u.email_option.option;\n+\n \tif (!email)\n \t\treturn xstrdup(\"\");\n-\tswitch (atom->u.email_option.option) {\n-\tcase EO_RAW:\n-\t\teoemail = strchr(email, '>');\n-\t\tif (eoemail)\n-\t\t\teoemail++;\n-\t\tbreak;\n-\tcase EO_TRIM:\n-\t\temail++;\n-\t\teoemail = strchr(email, '>');\n-\t\tbreak;\n-\tcase EO_LOCALPART:\n+\n+\tif (opt & (EO_LOCALPART | EO_TRIM))\n \t\temail++;\n-\t\teoemail = strchr(email, '@');\n-\t\tif (!eoemail)\n-\t\t\teoemail = strchr(email, '>');\n-\t\tbreak;\n-\tdefault:\n-\t\tBUG(\"unknown email option\");\n-\t}\n \n+\teoemail = find_end_of_email(email, opt);\n \tif (!eoemail)\n \t\treturn xstrdup(\"\");\n \treturn xmemdupz(email, eoemail - email);\n@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n \tv->value = 0;\n }\n \n+static struct string_list mailmap = STRING_LIST_INIT_NODUP;\n+\n /* See grab_values */\n static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tint wholen = strlen(who);\n \tconst char *wholine = NULL;\n+\tconst char *headers[] = { \"author \", \"committer \",\n+\t\t\t\t  \"tagger \", NULL };\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\t\tstruct strbuf mailmap_buf = STRBUF_INIT;\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\tif (strncmp(who, name, wholen))\n \t\t\tcontinue;\n \t\tif (name[wholen] != 0 &&\n-\t\t    strcmp(name + wholen, \"name\") &&\n+\t\t    !starts_with(name + wholen, \"name\") &&\n \t\t    !starts_with(name + wholen, \"email\") &&\n \t\t    !starts_with(name + wholen, \"date\"))\n \t\t\tcontinue;\n-\t\tif (!wholine)\n+\n+\t\tif ((starts_with(name + wholen, \"name\") &&\n+\t\t    (atom->u.name_option.option == N_MAILMAP)) ||\n+\t\t    (starts_with(name + wholen, \"email\") &&\n+\t\t    (atom->u.email_option.option & EO_MAILMAP))) {\n+\t\t\tif (!mailmap.items)\n+\t\t\t\tread_mailmap(&mailmap);\n+\t\t\tstrbuf_addstr(&mailmap_buf, buf);\n+\t\t\tapply_mailmap_to_header(&mailmap_buf, headers, &mailmap);\n+\t\t\twholine = find_wholine(who, wholen, mailmap_buf.buf);\n+\t\t} else {\n \t\t\twholine = find_wholine(who, wholen, buf);\n+\t\t}\n+\n \t\tif (!wholine)\n \t\t\treturn; /* no point looking for it */\n \t\tif (name[wholen] == 0)\n \t\t\tv->s = copy_line(wholine);\n-\t\telse if (!strcmp(name + wholen, \"name\"))\n+\t\telse if (starts_with(name + wholen, \"name\"))\n \t\t\tv->s = copy_name(wholine);\n \t\telse if (starts_with(name + wholen, \"email\"))\n \t\t\tv->s = copy_email(wholine, &used_atom[i]);\n \t\telse if (starts_with(name + wholen, \"date\"))\n \t\t\tgrab_date(wholine, v, name);\n+\n+\t\tstrbuf_release(&mailmap_buf);\n \t}\n \n \t/*\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 15b4622f57..1fb464ca50 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -25,6 +25,13 @@ test_expect_success setup '\n \tdisklen sha1:138\n \tdisklen sha256:154\n \tEOF\n+\n+\t# setup .mailmap\n+\tcat >.mailmap <<-EOF &&\n+\tA Thor <athor@example.com> A U Thor <author@example.com>\n+\tC Mitter <cmitter@example.com> C O Mitter <committer@example.com>\n+\tEOF\n+\n \tsetdate_and_increment &&\n \techo \"Using $datestamp\" > one &&\n \tgit add one &&\n@@ -141,15 +148,31 @@ test_atom head '*objectname' ''\n test_atom head '*objecttype' ''\n test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'\n test_atom head authorname 'A U Thor'\n+test_atom head authorname:mailmap 'A Thor'\n test_atom head authoremail '<author@example.com>'\n test_atom head authoremail:trim 'author@example.com'\n test_atom head authoremail:localpart 'author'\n+test_atom head authoremail:trim,localpart 'author'\n+test_atom head authoremail:mailmap '<athor@example.com>'\n+test_atom head authoremail:mailmap,trim 'athor@example.com'\n+test_atom head authoremail:trim,mailmap 'athor@example.com'\n+test_atom head authoremail:mailmap,localpart 'athor'\n+test_atom head authoremail:localpart,mailmap 'athor'\n+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'\n test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'\n test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'\n test_atom head committername 'C O Mitter'\n+test_atom head committername:mailmap 'C Mitter'\n test_atom head committeremail '<committer@example.com>'\n test_atom head committeremail:trim 'committer@example.com'\n test_atom head committeremail:localpart 'committer'\n+test_atom head committeremail:localpart,trim 'committer'\n+test_atom head committeremail:mailmap '<cmitter@example.com>'\n+test_atom head committeremail:mailmap,trim 'cmitter@example.com'\n+test_atom head committeremail:trim,mailmap 'cmitter@example.com'\n+test_atom head committeremail:mailmap,localpart 'cmitter'\n+test_atom head committeremail:localpart,mailmap 'cmitter'\n+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'\n test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'\n test_atom head tag ''\n test_atom head tagger ''\n@@ -199,22 +222,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})\n test_atom tag '*objecttype' 'commit'\n test_atom tag author ''\n test_atom tag authorname ''\n+test_atom tag authorname:mailmap ''\n test_atom tag authoremail ''\n test_atom tag authoremail:trim ''\n test_atom tag authoremail:localpart ''\n+test_atom tag authoremail:trim,localpart ''\n+test_atom tag authoremail:mailmap ''\n+test_atom tag authoremail:mailmap,trim ''\n+test_atom tag authoremail:trim,mailmap ''\n+test_atom tag authoremail:mailmap,localpart ''\n+test_atom tag authoremail:localpart,mailmap ''\n+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''\n test_atom tag authordate ''\n test_atom tag committer ''\n test_atom tag committername ''\n+test_atom tag committername:mailmap ''\n test_atom tag committeremail ''\n test_atom tag committeremail:trim ''\n test_atom tag committeremail:localpart ''\n+test_atom tag committeremail:localpart,trim ''\n+test_atom tag committeremail:mailmap ''\n+test_atom tag committeremail:mailmap,trim ''\n+test_atom tag committeremail:trim,mailmap ''\n+test_atom tag committeremail:mailmap,localpart ''\n+test_atom tag committeremail:localpart,mailmap ''\n+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''\n test_atom tag committerdate ''\n test_atom tag tag 'testtag'\n test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'\n test_atom tag taggername 'C O Mitter'\n+test_atom tag taggername:mailmap 'C Mitter'\n test_atom tag taggeremail '<committer@example.com>'\n test_atom tag taggeremail:trim 'committer@example.com'\n test_atom tag taggeremail:localpart 'committer'\n+test_atom tag taggeremail:trim,localpart 'committer'\n+test_atom tag taggeremail:mailmap '<cmitter@example.com>'\n+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'\n+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'\n+test_atom tag taggeremail:mailmap,localpart 'cmitter'\n+test_atom tag taggeremail:localpart,mailmap 'cmitter'\n+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'\n test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'\n test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'\n test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'\n@@ -284,9 +331,45 @@ test_bad_atom() {\n test_bad_atom head 'authoremail:foo' \\\n \t'fatal: unrecognized %(authoremail) argument: foo'\n \n+test_bad_atom head 'authoremail:mailmap,trim,bar' \\\n+\t'fatal: unrecognized %(authoremail) argument: bar'\n+\n+test_bad_atom head 'authoremail:trim,' \\\n+\t'fatal: unrecognized %(authoremail) argument: '\n+\n+test_bad_atom head 'authoremail:mailmaptrim' \\\n+\t'fatal: unrecognized %(authoremail) argument: trim'\n+\n+test_bad_atom head 'committeremail: ' \\\n+\t'fatal: unrecognized %(committeremail) argument:  '\n+\n+test_bad_atom head 'committeremail: trim,foo' \\\n+\t'fatal: unrecognized %(committeremail) argument:  trim,foo'\n+\n+test_bad_atom head 'committeremail:mailmap,localpart ' \\\n+\t'fatal: unrecognized %(committeremail) argument:  '\n+\n+test_bad_atom head 'committeremail:trim_localpart' \\\n+\t'fatal: unrecognized %(committeremail) argument: _localpart'\n+\n+test_bad_atom head 'committeremail:localpart,,,trim' \\\n+\t'fatal: unrecognized %(committeremail) argument: ,,trim'\n+\n+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \\\n+\t'fatal: unrecognized %(taggeremail) argument:  foo '\n+\n+test_bad_atom tag 'taggeremail:trim,localpart,' \\\n+\t'fatal: unrecognized %(taggeremail) argument: '\n+\n+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument: ;localpart trim'\n+\n test_bad_atom tag 'taggeremail:localpart trim' \\\n \t'fatal: unrecognized %(taggeremail) argument:  trim'\n \n+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'\n+\n test_date () {\n \tf=$1 &&\n \tcommitter_date=$2 &&\n-- \n2.42.0.160.g6905eb16ce.dirty\n\n"},{"id":"482089","messageId":"xmqqy1h078tf.fsf@gitster.g","threadId":"60251","inReplyTo":"20230920191654.6133-2-five231003@gmail.com","subject":"Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-20T22:56:28Z","receivedAt":"2023-09-20T22:56:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kousik Sanagavarapu <five231003@gmail.com> writes:\n\n> Introduce a new function \"test_bad_atom()\", which is similar to\n> \"test_atom()\" but should be used to check whether the correct error\n> message is shown on stderr.\n>\n> Like \"test_atom()\", the new function takes three arguments. The three\n> arguments specify the ref, the format and the expected error message\n> respectively, with an optional fourth argument for tweaking\n> \"test_expect_*\" (which is by default \"success\").\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Hariom Verma <hariom18599@gmail.com>\n> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n> ---\n>  t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++\n>  1 file changed, 20 insertions(+)\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 7b943fd34c..15b4622f57 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers\n>  \ttest_must_fail git for-each-ref --format=\"%(objectname:short=foo)\"\n>  '\n>  \n> +test_bad_atom() {\n\nStyle: have SP on both sides of \"()\".\n\n> +\tcase \"$1\" in\n> +\thead) ref=refs/heads/main ;;\n> +\t tag) ref=refs/tags/testtag ;;\n> +\t sym) ref=refs/heads/sym ;;\n> +\t   *) ref=$1 ;;\n> +\tesac\n\nSomehow this indirection makes the two examples we see below harder\nto understand.  Why shouldn't we just write the full refname on th\ncommand line of test_bad_atom?  That would make it crystal clear\nwhich ref each test works on.  It does not help that both 'head' and\n'sym' refer to a local branch (if the former referred to .git/HEAD\nor .git/remotes/origin/HEAD it may have been an excellent choice of\nthe name, but that is not what is going on).\n\n> +\tprintf '%s\\n' \"$3\">expect\n\nStyle: need SP before (but not after) '>'.\n\n> +\ttest_expect_${4:-success} $PREREQ \"err basic atom: $1 $2\" \"\n> +\t\ttest_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&\n> +\t\ttest_cmp expect actual\n> +\t\"\n\nIt is error prone to have the executable part of test_expect_{success,failure}\ninside a pair of double quotes and have $variable interpolated\n_before_ even the arguments to test_expect_{success,failure} are\nformed.  It is much more preferrable to write\n\n\ttest_bad_atom () {\n\t\tref=$1 format=$2\n\t\tprintf '%s\\n' \"$3\" >expect\n\t\t$test_do=test_expect_${4:-success}\n\n\t\t$test_do $PREREQ \"err basic atom: $ref $format\" '\n\t\t\ttest_must_fail git for-each-ref \\\n\t\t\t\t--format=\"%($format)\" \"$ref\" 2>error &&\n\t\t\ttest_cmp expect error\n\t\t'\n\t}\n\nThis is primarily because you cannot control what is in \"$2\" to\nensure the correctness of the test we see above locally (i.e. if\nyour caller has a single-quote in \"$2\", the shell script you create\nfor running test_expect_{success,failure} would be syntactically\nincorrect).  By enclosing the executable part inside a pair of\nsingle quotes, and having the $variables interpolated when that\nexecutable part is `eval`ed when test_expect_{success,failure} runs,\nyou will avoid such problems, and those reading the above can locally\nknow that you are aware of and correctly avoiding such problems.\n\nI guess three among four problems I just pointed out you blindly\ncopied from test_atom.  But let's not spread badness (preliminary\nclean-up to existing badness would be welcome instead).\n\n> +}\n> +\n> +test_bad_atom head 'authoremail:foo' \\\n> +\t'fatal: unrecognized %(authoremail) argument: foo'\n> +\n> +test_bad_atom tag 'taggeremail:localpart trim' \\\n> +\t'fatal: unrecognized %(taggeremail) argument:  trim'\n\nIt is strange to see double SP before 'trim' in this error message.\nAre we etching a code mistake in stone here?  Wouldn't the error\nmessage say \"...argument: localpart trim\" instead, perhaps?\n\n>  test_date () {\n>  \tf=$1 &&\n>  \tcommitter_date=$2 &&\n"},{"id":"482090","messageId":"xmqqttro787y.fsf@gitster.g","threadId":"60251","inReplyTo":"xmqqy1h078tf.fsf@gitster.g","subject":"Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-20T23:09:21Z","receivedAt":"2023-09-20T23:09:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +\tcase \"$1\" in\n>> +\thead) ref=refs/heads/main ;;\n>> +\t tag) ref=refs/tags/testtag ;;\n>> +\t sym) ref=refs/heads/sym ;;\n>> +\t   *) ref=$1 ;;\n>> +\tesac\n>\n> Somehow this indirection makes the two examples we see below harder\n> to understand.  ... It does not help that both 'head' and\n> 'sym' refer to a local branch ...\n\nAh, this \"sym\" thing is a (rather unnatural) symbolic ref inside\nrefs/heads/ hierarchy, so the naming makes halfway sense.  As it is\nalso used in test_atom, I no longer find it all that much disturbing.\n\nEverything else I said in my review still stands, I would think.\n\nThanks.\n"},{"id":"482091","messageId":"xmqqmsxg77nh.fsf@gitster.g","threadId":"60251","inReplyTo":"xmqqy1h078tf.fsf@gitster.g","subject":"Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-20T23:21:38Z","receivedAt":"2023-09-20T23:21:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +test_bad_atom tag 'taggeremail:localpart trim' \\\n>> +\t'fatal: unrecognized %(taggeremail) argument:  trim'\n>\n> It is strange to see double SP before 'trim' in this error message.\n> Are we etching a code mistake in stone here?  Wouldn't the error\n> message say \"...argument: localpart trim\" instead, perhaps?\n\nI tried.  The fatal message does say ...argument: localpart trim\" as\nI suspected, when you ask for 'taggeremail:localpart trim'.\n\nI think I know what is going on.  With the [PATCH 1/2] as-is, this\npiece does not pass.  But because the error message from parsing\ngets broken by [PATCH 2/2], after applying [PATCH 2/2], the error\nmessage will become what the above test expects, hiding the new\nbreakage in the code.  And it probably was not noticed before you\nsent the patches, because you did not test [PATCH 1/2] alone.\n\n\n\n\n"},{"id":"482103","messageId":"ZQySKnKEirmhXN-U@five231003","threadId":"60251","inReplyTo":"xmqqy1h078tf.fsf@gitster.g","subject":"Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-21T18:57:46Z","receivedAt":"2023-09-21T20:01:13Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Sorry for the late reply.\n\nOn Wed, Sep 20, 2023 at 03:56:28PM -0700, Junio C Hamano wrote:\n> Kousik Sanagavarapu <five231003@gmail.com> writes:\n> \n> > Introduce a new function \"test_bad_atom()\", which is similar to\n> > \"test_atom()\" but should be used to check whether the correct error\n> > message is shown on stderr.\n> >\n> > Like \"test_atom()\", the new function takes three arguments. The three\n> > arguments specify the ref, the format and the expected error message\n> > respectively, with an optional fourth argument for tweaking\n> > \"test_expect_*\" (which is by default \"success\").\n> >\n> > Mentored-by: Christian Couder <christian.couder@gmail.com>\n> > Mentored-by: Hariom Verma <hariom18599@gmail.com>\n> > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n> > ---\n> >  t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++\n> >  1 file changed, 20 insertions(+)\n> >\n> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> > index 7b943fd34c..15b4622f57 100755\n> > --- a/t/t6300-for-each-ref.sh\n> > +++ b/t/t6300-for-each-ref.sh\n> > @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers\n> >  \ttest_must_fail git for-each-ref --format=\"%(objectname:short=foo)\"\n> >  '\n> >  \n> > +test_bad_atom() {\n> \n> Style: have SP on both sides of \"()\".\n> \n> [...]\n>\n> > +\tprintf '%s\\n' \"$3\">expect\n> \n> Style: need SP before (but not after) '>'.\n\nI'll make these style changes, they slipped by.\n\n> > +\ttest_expect_${4:-success} $PREREQ \"err basic atom: $1 $2\" \"\n> > +\t\ttest_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&\n> > +\t\ttest_cmp expect actual\n> > +\t\"\n> \n> It is error prone to have the executable part of test_expect_{success,failure}\n> inside a pair of double quotes and have $variable interpolated\n> _before_ even the arguments to test_expect_{success,failure} are\n> formed.  It is much more preferrable to write\n> \n> \ttest_bad_atom () {\n> \t\tref=$1 format=$2\n> \t\tprintf '%s\\n' \"$3\" >expect\n> \t\t$test_do=test_expect_${4:-success}\n> \n> \t\t$test_do $PREREQ \"err basic atom: $ref $format\" '\n> \t\t\ttest_must_fail git for-each-ref \\\n> \t\t\t\t--format=\"%($format)\" \"$ref\" 2>error &&\n> \t\t\ttest_cmp expect error\n> \t\t'\n> \t}\n> \n> This is primarily because you cannot control what is in \"$2\" to\n> ensure the correctness of the test we see above locally (i.e. if\n> your caller has a single-quote in \"$2\", the shell script you create\n> for running test_expect_{success,failure} would be syntactically\n> incorrect).  By enclosing the executable part inside a pair of\n> single quotes, and having the $variables interpolated when that\n> executable part is `eval`ed when test_expect_{success,failure} runs,\n> you will avoid such problems, and those reading the above can locally\n> know that you are aware of and correctly avoiding such problems.\n\nI see.\n\n> I guess three among four problems I just pointed out you blindly\n> copied from test_atom.  But let's not spread badness (preliminary\n> clean-up to existing badness would be welcome instead).\n\nYeah, I had copied it from test_atom. Although I didn't realize that it\nwas bad to implement test_bad_atom the way I did. Thanks for such a nice\nexplanation. So I guess we can include the test_atom cleanup in this\nseries?\n\n> > +}\n> > +\n> > +test_bad_atom head 'authoremail:foo' \\\n> > +\t'fatal: unrecognized %(authoremail) argument: foo'\n> > +\n> > +test_bad_atom tag 'taggeremail:localpart trim' \\\n> > +\t'fatal: unrecognized %(taggeremail) argument:  trim'\n> \n> It is strange to see double SP before 'trim' in this error message.\n> Are we etching a code mistake in stone here?  Wouldn't the error\n> message say \"...argument: localpart trim\" instead, perhaps?\n> \n> >  test_date () {\n> >  \tf=$1 &&\n> >  \tcommitter_date=$2 &&\n\nSo I read the the other replies and it seems that it indeed hides a\nbreakage and yeah I hadn't tested PATCH 1/2 independently. I'll change\nthis too.\n\nThanks\n"},{"id":"482290","messageId":"20230925175050.3498-1-five231003@gmail.com","threadId":"60251","inReplyTo":"20230920191654.6133-1-five231003@gmail.com","subject":"[PATCH v2 0/3] Add mailmap support to ref-filter","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-25T17:43:07Z","receivedAt":"2023-09-25T17:51:02Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Thanks Junio for review on the previous round.\n\nPATCH 1/3 - Cleanup test_atom to be less error-prone\n\nPATCH 2/3 - Fix hidden breakage\n\nPATCH 3/3 - Unchanged\n\nKousik Sanagavarapu (3):\n  t/t6300: cleanup test_atom\n  t/t6300: introduce test_bad_atom\n  ref-filter: add mailmap support\n\n Documentation/git-for-each-ref.txt |   6 +-\n ref-filter.c                       | 152 ++++++++++++++++++++++-------\n t/t6300-for-each-ref.sh            | 123 +++++++++++++++++++++--\n 3 files changed, 239 insertions(+), 42 deletions(-)\n\nRange-diff against v1:\n\n-:  ---------- > 1:  b28e858e35 t/t6300: cleanup test_atom\n1:  0a90b67889 ! 2:  ee90d017d5 t/t6300: introduce test_bad_atom()\n    @@ Metadata\n     Author: Kousik Sanagavarapu <five231003@gmail.com>\n\n      ## Commit message ##\n    -    t/t6300: introduce test_bad_atom()\n    +    t/t6300: introduce test_bad_atom\n\n    -    Introduce a new function \"test_bad_atom()\", which is similar to\n    +    Introduce a new function \"test_bad_atom\", which is similar to\n         \"test_atom()\" but should be used to check whether the correct error\n         message is shown on stderr.\n\n    -    Like \"test_atom()\", the new function takes three arguments. The three\n    +    Like \"test_atom\", the new function takes three arguments. The three\n         arguments specify the ref, the format and the expected error message\n         respectively, with an optional fourth argument for tweaking\n         \"test_expect_*\" (which is by default \"success\").\n    @@ t/t6300-for-each-ref.sh: test_expect_success 'arguments to %(objectname:short=)\n        test_must_fail git for-each-ref --format=\"%(objectname:short=foo)\"\n      '\n\n    -+test_bad_atom() {\n    ++test_bad_atom () {\n     +  case \"$1\" in\n     +  head) ref=refs/heads/main ;;\n     +   tag) ref=refs/tags/testtag ;;\n     +   sym) ref=refs/heads/sym ;;\n     +     *) ref=$1 ;;\n     +  esac\n    -+  printf '%s\\n' \"$3\">expect\n    -+  test_expect_${4:-success} $PREREQ \"err basic atom: $1 $2\" \"\n    -+          test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&\n    -+          test_cmp expect actual\n    -+  \"\n    ++  format=$2\n    ++  test_do=test_expect_${4:-success}\n    ++\n    ++  printf '%s\\n' \"$3\" >expect\n    ++  $test_do $PREREQ \"err basic atom: $ref $format\" '\n    ++          test_must_fail git for-each-ref \\\n    ++                  --format=\"%($format)\" \"$ref\" 2>error &&\n    ++          test_cmp expect error\n    ++  '\n     +}\n     +\n     +test_bad_atom head 'authoremail:foo' \\\n     +  'fatal: unrecognized %(authoremail) argument: foo'\n     +\n     +test_bad_atom tag 'taggeremail:localpart trim' \\\n    -+  'fatal: unrecognized %(taggeremail) argument:  trim'\n    ++  'fatal: unrecognized %(taggeremail) argument: localpart trim'\n     +\n      test_date () {\n        f=$1 &&\n2:  63fc69f4dc ! 3:  fdc14fe80b ref-filter: add mailmap support\n    @@ t/t6300-for-each-ref.sh: test_atom tag '*objectname' $(git rev-parse refs/tags/t\n      test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'\n      test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'\n      test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'\n    -@@ t/t6300-for-each-ref.sh: test_bad_atom() {\n    +@@ t/t6300-for-each-ref.sh: test_bad_atom () {\n      test_bad_atom head 'authoremail:foo' \\\n        'fatal: unrecognized %(authoremail) argument: foo'\n\n    @@ t/t6300-for-each-ref.sh: test_bad_atom() {\n     +  'fatal: unrecognized %(taggeremail) argument: ;localpart trim'\n     +\n      test_bad_atom tag 'taggeremail:localpart trim' \\\n    -   'fatal: unrecognized %(taggeremail) argument:  trim'\n    -\n    +-  'fatal: unrecognized %(taggeremail) argument: localpart trim'\n    ++  'fatal: unrecognized %(taggeremail) argument:  trim'\n    ++\n     +test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \\\n     +  'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'\n    -+\n    +\n      test_date () {\n        f=$1 &&\n    -   committer_date=$2 &&\n\n-- \n2.42.0.273.ge948a9aaf4\n\n"},{"id":"482291","messageId":"20230925175050.3498-2-five231003@gmail.com","threadId":"60251","inReplyTo":"20230925175050.3498-1-five231003@gmail.com","subject":"[PATCH v2 1/3] t/t6300: cleanup test_atom","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-25T17:43:08Z","receivedAt":"2023-09-25T17:51:06Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Previously, when the executable part of \"test_expect_{success,failure}\"\n(inside \"test_atom\") got \"eval\"ed, it would have been syntactically\nincorrect if the second argument ($2, which is the format) to \"test_atom\"\nwere enclosed in single quotes because the $variables would get\ninterpolated even before the arguments to \"test_expect_{success,failure}\"\nare formed.\n\nSo fix this and also some style issues along the way.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n---\n t/t6300-for-each-ref.sh | 16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 7b943fd34c..7ba9949376 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -41,25 +41,29 @@ test_expect_success setup '\n \tgit config push.default current\n '\n \n-test_atom() {\n+test_atom () {\n \tcase \"$1\" in\n \t\thead) ref=refs/heads/main ;;\n \t\t tag) ref=refs/tags/testtag ;;\n \t\t sym) ref=refs/heads/sym ;;\n \t\t   *) ref=$1 ;;\n \tesac\n+\tformat=$2\n+\ttest_do=test_expect_${4:-success}\n+\n \tprintf '%s\\n' \"$3\" >expected\n-\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 $2\" \"\n-\t\tgit for-each-ref --format='%($2)' $ref >actual &&\n+\t$test_do $PREREQ \"basic atom: $ref $format\" '\n+\t\tgit for-each-ref --format=\"%($format)\" \"$ref\" >actual &&\n \t\tsanitize_pgp <actual >actual.clean &&\n \t\ttest_cmp expected actual.clean\n-\t\"\n+\t'\n+\n \t# Automatically test \"contents:size\" atom after testing \"contents\"\n-\tif test \"$2\" = \"contents\"\n+\tif test \"$format\" = \"contents\"\n \tthen\n \t\t# for commit leg, $3 is changed there\n \t\texpect=$(printf '%s' \"$3\" | wc -c)\n-\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\t$test_do $PREREQ \"basic atom: $ref contents:size\" '\n \t\t\ttype=$(git cat-file -t \"$ref\") &&\n \t\t\tcase $type in\n \t\t\ttag)\n-- \n2.42.0.273.ge948a9aaf4\n\n"},{"id":"482292","messageId":"20230925175050.3498-3-five231003@gmail.com","threadId":"60251","inReplyTo":"20230925175050.3498-1-five231003@gmail.com","subject":"[PATCH v2 2/3] t/t6300: introduce test_bad_atom","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-25T17:43:09Z","receivedAt":"2023-09-25T17:51:11Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Introduce a new function \"test_bad_atom\", which is similar to\n\"test_atom()\" but should be used to check whether the correct error\nmessage is shown on stderr.\n\nLike \"test_atom\", the new function takes three arguments. The three\narguments specify the ref, the format and the expected error message\nrespectively, with an optional fourth argument for tweaking\n\"test_expect_*\" (which is by default \"success\").\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n---\n t/t6300-for-each-ref.sh | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 7ba9949376..e4ec2926d6 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -271,6 +271,30 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers\n \ttest_must_fail git for-each-ref --format=\"%(objectname:short=foo)\"\n '\n \n+test_bad_atom () {\n+\tcase \"$1\" in\n+\thead) ref=refs/heads/main ;;\n+\t tag) ref=refs/tags/testtag ;;\n+\t sym) ref=refs/heads/sym ;;\n+\t   *) ref=$1 ;;\n+\tesac\n+\tformat=$2\n+\ttest_do=test_expect_${4:-success}\n+\n+\tprintf '%s\\n' \"$3\" >expect\n+\t$test_do $PREREQ \"err basic atom: $ref $format\" '\n+\t\ttest_must_fail git for-each-ref \\\n+\t\t\t--format=\"%($format)\" \"$ref\" 2>error &&\n+\t\ttest_cmp expect error\n+\t'\n+}\n+\n+test_bad_atom head 'authoremail:foo' \\\n+\t'fatal: unrecognized %(authoremail) argument: foo'\n+\n+test_bad_atom tag 'taggeremail:localpart trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument: localpart trim'\n+\n test_date () {\n \tf=$1 &&\n \tcommitter_date=$2 &&\n-- \n2.42.0.273.ge948a9aaf4\n\n"},{"id":"482293","messageId":"20230925175050.3498-4-five231003@gmail.com","threadId":"60251","inReplyTo":"20230925175050.3498-1-five231003@gmail.com","subject":"[PATCH v2 3/3] ref-filter: add mailmap support","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-09-25T17:43:10Z","receivedAt":"2023-09-25T17:51:17Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Add mailmap support to ref-filter formats which are similar in\npretty. This support is such that the following pretty placeholders are\nequivalent to the new ref-filter atoms:\n\n\t%aN = authorname:mailmap\n\t%cN = committername:mailmap\n\n\t%aE = authoremail:mailmap\n\t%aL = authoremail:mailmap,localpart\n\t%cE = committeremail:mailmap\n\t%cL = committeremail:mailmap,localpart\n\nAdditionally, mailmap can also be used with \":trim\" option for email by\ndoing something like \"authoremail:mailmap,trim\".\n\nThe above also applies for the \"tagger\" atom, that is,\n\"taggername:mailmap\", \"taggeremail:mailmap\", \"taggeremail:mailmap,trim\"\nand \"taggername:mailmap,localpart\".\n\nThe functionality of \":trim\" and \":localpart\" remains the same. That is,\n\":trim\" gives the email, but without the angle brackets and \":localpart\"\ngives the part of the email before the '@' character (if such a\ncharacter is not found then we directly grab everything between the\nangle brackets).\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: Kousik Sanagavarapu <five231003@gmail.com>\n---\n Documentation/git-for-each-ref.txt |   6 +-\n ref-filter.c                       | 152 ++++++++++++++++++++++-------\n t/t6300-for-each-ref.sh            |  85 +++++++++++++++-\n 3 files changed, 206 insertions(+), 37 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 11b2bc3121..e86d5700dd 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,\n and `date` to extract the named component.  For email fields (`authoremail`,\n `committeremail` and `taggeremail`), `:trim` can be appended to get the email\n without angle brackets, and `:localpart` to get the part before the `@` symbol\n-out of the trimmed email.\n+out of the trimmed email. In addition to these, the `:mailmap` option and the\n+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does\n+not matter) to get values of the name and email according to the .mailmap file\n+or according to the file set in the mailmap.file or mailmap.blob configuration\n+variable (see linkgit:gitmailmap[5]).\n \n The raw data in an object is `raw`.\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex fae9f4b8ed..e4d3510e28 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -13,6 +13,8 @@\n #include \"oid-array.h\"\n #include \"repository.h\"\n #include \"commit.h\"\n+#include \"mailmap.h\"\n+#include \"ident.h\"\n #include \"remote.h\"\n #include \"color.h\"\n #include \"tag.h\"\n@@ -215,8 +217,16 @@ static struct used_atom {\n \t\tstruct {\n \t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n \t\t} objectsize;\n-\t\tstruct email_option {\n-\t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n+\t\tstruct {\n+\t\t\tenum { N_RAW, N_MAILMAP } option;\n+\t\t} name_option;\n+\t\tstruct {\n+\t\t\tenum {\n+\t\t\t\tEO_RAW = 0,\n+\t\t\t\tEO_TRIM = 1<<0,\n+\t\t\t\tEO_LOCALPART = 1<<1,\n+\t\t\t\tEO_MAILMAP = 1<<2,\n+\t\t\t} option;\n \t\t} email_option;\n \t\tstruct {\n \t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY,\n@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,\n \treturn 0;\n }\n \n-static int person_email_atom_parser(struct ref_format *format UNUSED,\n-\t\t\t\t    struct used_atom *atom,\n-\t\t\t\t    const char *arg, struct strbuf *err)\n+static int person_name_atom_parser(struct ref_format *format UNUSED,\n+\t\t\t\t   struct used_atom *atom,\n+\t\t\t\t   const char *arg, struct strbuf *err)\n {\n \tif (!arg)\n-\t\tatom->u.email_option.option = EO_RAW;\n-\telse if (!strcmp(arg, \"trim\"))\n-\t\tatom->u.email_option.option = EO_TRIM;\n-\telse if (!strcmp(arg, \"localpart\"))\n-\t\tatom->u.email_option.option = EO_LOCALPART;\n+\t\tatom->u.name_option.option = N_RAW;\n+\telse if (!strcmp(arg, \"mailmap\"))\n+\t\tatom->u.name_option.option = N_MAILMAP;\n \telse\n \t\treturn err_bad_arg(err, atom->name, arg);\n \treturn 0;\n }\n \n+static int email_atom_option_parser(struct used_atom *atom,\n+\t\t\t\t    const char **arg, struct strbuf *err)\n+{\n+\tif (!*arg)\n+\t\treturn EO_RAW;\n+\tif (skip_prefix(*arg, \"trim\", arg))\n+\t\treturn EO_TRIM;\n+\tif (skip_prefix(*arg, \"localpart\", arg))\n+\t\treturn EO_LOCALPART;\n+\tif (skip_prefix(*arg, \"mailmap\", arg))\n+\t\treturn EO_MAILMAP;\n+\treturn -1;\n+}\n+\n+static int person_email_atom_parser(struct ref_format *format UNUSED,\n+\t\t\t\t    struct used_atom *atom,\n+\t\t\t\t    const char *arg, struct strbuf *err)\n+{\n+\tfor (;;) {\n+\t\tint opt = email_atom_option_parser(atom, &arg, err);\n+\t\tconst char *bad_arg = arg;\n+\n+\t\tif (opt < 0)\n+\t\t\treturn err_bad_arg(err, atom->name, bad_arg);\n+\t\tatom->u.email_option.option |= opt;\n+\n+\t\tif (!arg || !*arg)\n+\t\t\tbreak;\n+\t\tif (*arg == ',')\n+\t\t\targ++;\n+\t\telse\n+\t\t\treturn err_bad_arg(err, atom->name, bad_arg);\n+\t}\n+\treturn 0;\n+}\n+\n static int refname_atom_parser(struct ref_format *format UNUSED,\n \t\t\t       struct used_atom *atom,\n \t\t\t       const char *arg, struct strbuf *err)\n@@ -877,15 +921,15 @@ static struct {\n \t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n \t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n \t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n-\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n+\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n-\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n+\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n-\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n+\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },\n \t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n \t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n \t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)\n \treturn xstrdup(\"\");\n }\n \n+static const char *find_end_of_email(const char *email, int opt)\n+{\n+\tconst char *eoemail;\n+\n+\tif (opt & EO_LOCALPART) {\n+\t\teoemail = strchr(email, '@');\n+\t\tif (eoemail)\n+\t\t\treturn eoemail;\n+\t\treturn strchr(email, '>');\n+\t}\n+\n+\tif (opt & EO_TRIM)\n+\t\treturn strchr(email, '>');\n+\n+\t/*\n+\t * The option here is either the raw email option or the raw\n+\t * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,\n+\t * we directly grab the whole email including the closing\n+\t * angle brackets.\n+\t *\n+\t * If EO_MAILMAP was set with any other option (that is either\n+\t * EO_TRIM or EO_LOCALPART), we already grab the end of email\n+\t * above.\n+\t */\n+\teoemail = strchr(email, '>');\n+\tif (eoemail)\n+\t\teoemail++;\n+\treturn eoemail;\n+}\n+\n static const char *copy_email(const char *buf, struct used_atom *atom)\n {\n \tconst char *email = strchr(buf, '<');\n \tconst char *eoemail;\n+\tint opt = atom->u.email_option.option;\n+\n \tif (!email)\n \t\treturn xstrdup(\"\");\n-\tswitch (atom->u.email_option.option) {\n-\tcase EO_RAW:\n-\t\teoemail = strchr(email, '>');\n-\t\tif (eoemail)\n-\t\t\teoemail++;\n-\t\tbreak;\n-\tcase EO_TRIM:\n-\t\temail++;\n-\t\teoemail = strchr(email, '>');\n-\t\tbreak;\n-\tcase EO_LOCALPART:\n+\n+\tif (opt & (EO_LOCALPART | EO_TRIM))\n \t\temail++;\n-\t\teoemail = strchr(email, '@');\n-\t\tif (!eoemail)\n-\t\t\teoemail = strchr(email, '>');\n-\t\tbreak;\n-\tdefault:\n-\t\tBUG(\"unknown email option\");\n-\t}\n \n+\teoemail = find_end_of_email(email, opt);\n \tif (!eoemail)\n \t\treturn xstrdup(\"\");\n \treturn xmemdupz(email, eoemail - email);\n@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n \tv->value = 0;\n }\n \n+static struct string_list mailmap = STRING_LIST_INIT_NODUP;\n+\n /* See grab_values */\n static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tint wholen = strlen(who);\n \tconst char *wholine = NULL;\n+\tconst char *headers[] = { \"author \", \"committer \",\n+\t\t\t\t  \"tagger \", NULL };\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\t\tstruct strbuf mailmap_buf = STRBUF_INIT;\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\tif (strncmp(who, name, wholen))\n \t\t\tcontinue;\n \t\tif (name[wholen] != 0 &&\n-\t\t    strcmp(name + wholen, \"name\") &&\n+\t\t    !starts_with(name + wholen, \"name\") &&\n \t\t    !starts_with(name + wholen, \"email\") &&\n \t\t    !starts_with(name + wholen, \"date\"))\n \t\t\tcontinue;\n-\t\tif (!wholine)\n+\n+\t\tif ((starts_with(name + wholen, \"name\") &&\n+\t\t    (atom->u.name_option.option == N_MAILMAP)) ||\n+\t\t    (starts_with(name + wholen, \"email\") &&\n+\t\t    (atom->u.email_option.option & EO_MAILMAP))) {\n+\t\t\tif (!mailmap.items)\n+\t\t\t\tread_mailmap(&mailmap);\n+\t\t\tstrbuf_addstr(&mailmap_buf, buf);\n+\t\t\tapply_mailmap_to_header(&mailmap_buf, headers, &mailmap);\n+\t\t\twholine = find_wholine(who, wholen, mailmap_buf.buf);\n+\t\t} else {\n \t\t\twholine = find_wholine(who, wholen, buf);\n+\t\t}\n+\n \t\tif (!wholine)\n \t\t\treturn; /* no point looking for it */\n \t\tif (name[wholen] == 0)\n \t\t\tv->s = copy_line(wholine);\n-\t\telse if (!strcmp(name + wholen, \"name\"))\n+\t\telse if (starts_with(name + wholen, \"name\"))\n \t\t\tv->s = copy_name(wholine);\n \t\telse if (starts_with(name + wholen, \"email\"))\n \t\t\tv->s = copy_email(wholine, &used_atom[i]);\n \t\telse if (starts_with(name + wholen, \"date\"))\n \t\t\tgrab_date(wholine, v, name);\n+\n+\t\tstrbuf_release(&mailmap_buf);\n \t}\n \n \t/*\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex e4ec2926d6..00a060df0b 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -25,6 +25,13 @@ test_expect_success setup '\n \tdisklen sha1:138\n \tdisklen sha256:154\n \tEOF\n+\n+\t# setup .mailmap\n+\tcat >.mailmap <<-EOF &&\n+\tA Thor <athor@example.com> A U Thor <author@example.com>\n+\tC Mitter <cmitter@example.com> C O Mitter <committer@example.com>\n+\tEOF\n+\n \tsetdate_and_increment &&\n \techo \"Using $datestamp\" > one &&\n \tgit add one &&\n@@ -145,15 +152,31 @@ test_atom head '*objectname' ''\n test_atom head '*objecttype' ''\n test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'\n test_atom head authorname 'A U Thor'\n+test_atom head authorname:mailmap 'A Thor'\n test_atom head authoremail '<author@example.com>'\n test_atom head authoremail:trim 'author@example.com'\n test_atom head authoremail:localpart 'author'\n+test_atom head authoremail:trim,localpart 'author'\n+test_atom head authoremail:mailmap '<athor@example.com>'\n+test_atom head authoremail:mailmap,trim 'athor@example.com'\n+test_atom head authoremail:trim,mailmap 'athor@example.com'\n+test_atom head authoremail:mailmap,localpart 'athor'\n+test_atom head authoremail:localpart,mailmap 'athor'\n+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'\n test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'\n test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'\n test_atom head committername 'C O Mitter'\n+test_atom head committername:mailmap 'C Mitter'\n test_atom head committeremail '<committer@example.com>'\n test_atom head committeremail:trim 'committer@example.com'\n test_atom head committeremail:localpart 'committer'\n+test_atom head committeremail:localpart,trim 'committer'\n+test_atom head committeremail:mailmap '<cmitter@example.com>'\n+test_atom head committeremail:mailmap,trim 'cmitter@example.com'\n+test_atom head committeremail:trim,mailmap 'cmitter@example.com'\n+test_atom head committeremail:mailmap,localpart 'cmitter'\n+test_atom head committeremail:localpart,mailmap 'cmitter'\n+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'\n test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'\n test_atom head tag ''\n test_atom head tagger ''\n@@ -203,22 +226,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})\n test_atom tag '*objecttype' 'commit'\n test_atom tag author ''\n test_atom tag authorname ''\n+test_atom tag authorname:mailmap ''\n test_atom tag authoremail ''\n test_atom tag authoremail:trim ''\n test_atom tag authoremail:localpart ''\n+test_atom tag authoremail:trim,localpart ''\n+test_atom tag authoremail:mailmap ''\n+test_atom tag authoremail:mailmap,trim ''\n+test_atom tag authoremail:trim,mailmap ''\n+test_atom tag authoremail:mailmap,localpart ''\n+test_atom tag authoremail:localpart,mailmap ''\n+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''\n test_atom tag authordate ''\n test_atom tag committer ''\n test_atom tag committername ''\n+test_atom tag committername:mailmap ''\n test_atom tag committeremail ''\n test_atom tag committeremail:trim ''\n test_atom tag committeremail:localpart ''\n+test_atom tag committeremail:localpart,trim ''\n+test_atom tag committeremail:mailmap ''\n+test_atom tag committeremail:mailmap,trim ''\n+test_atom tag committeremail:trim,mailmap ''\n+test_atom tag committeremail:mailmap,localpart ''\n+test_atom tag committeremail:localpart,mailmap ''\n+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''\n test_atom tag committerdate ''\n test_atom tag tag 'testtag'\n test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'\n test_atom tag taggername 'C O Mitter'\n+test_atom tag taggername:mailmap 'C Mitter'\n test_atom tag taggeremail '<committer@example.com>'\n test_atom tag taggeremail:trim 'committer@example.com'\n test_atom tag taggeremail:localpart 'committer'\n+test_atom tag taggeremail:trim,localpart 'committer'\n+test_atom tag taggeremail:mailmap '<cmitter@example.com>'\n+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'\n+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'\n+test_atom tag taggeremail:mailmap,localpart 'cmitter'\n+test_atom tag taggeremail:localpart,mailmap 'cmitter'\n+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'\n test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'\n test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'\n test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'\n@@ -292,8 +339,44 @@ test_bad_atom () {\n test_bad_atom head 'authoremail:foo' \\\n \t'fatal: unrecognized %(authoremail) argument: foo'\n \n+test_bad_atom head 'authoremail:mailmap,trim,bar' \\\n+\t'fatal: unrecognized %(authoremail) argument: bar'\n+\n+test_bad_atom head 'authoremail:trim,' \\\n+\t'fatal: unrecognized %(authoremail) argument: '\n+\n+test_bad_atom head 'authoremail:mailmaptrim' \\\n+\t'fatal: unrecognized %(authoremail) argument: trim'\n+\n+test_bad_atom head 'committeremail: ' \\\n+\t'fatal: unrecognized %(committeremail) argument:  '\n+\n+test_bad_atom head 'committeremail: trim,foo' \\\n+\t'fatal: unrecognized %(committeremail) argument:  trim,foo'\n+\n+test_bad_atom head 'committeremail:mailmap,localpart ' \\\n+\t'fatal: unrecognized %(committeremail) argument:  '\n+\n+test_bad_atom head 'committeremail:trim_localpart' \\\n+\t'fatal: unrecognized %(committeremail) argument: _localpart'\n+\n+test_bad_atom head 'committeremail:localpart,,,trim' \\\n+\t'fatal: unrecognized %(committeremail) argument: ,,trim'\n+\n+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \\\n+\t'fatal: unrecognized %(taggeremail) argument:  foo '\n+\n+test_bad_atom tag 'taggeremail:trim,localpart,' \\\n+\t'fatal: unrecognized %(taggeremail) argument: '\n+\n+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument: ;localpart trim'\n+\n test_bad_atom tag 'taggeremail:localpart trim' \\\n-\t'fatal: unrecognized %(taggeremail) argument: localpart trim'\n+\t'fatal: unrecognized %(taggeremail) argument:  trim'\n+\n+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \\\n+\t'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'\n \n test_date () {\n \tf=$1 &&\n-- \n2.42.0.273.ge948a9aaf4\n\n"}]}