threads / patch / 60251

patch, 2 partsAdd mailmap support to ref-filter

Subject: [PATCH 0/2] Add mailmap support to ref-filter

## tl;dr

11 messages between Sep 20, 2023 and Sep 25, 2023. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Kousik Sanagavarapu· Sep 20, 2023, 19:05 UTC · lore

Add mailmap support to ref-filter, making ref-filter and pretty closer, which is a part of the effort made to unify both ref-filter and pretty.

PATCH 1/2 - Introduces the "test_bad_atom()" function which checks for if the
	    given error message (either due to "err_bad_arg()" or any other err)
	    is correct.
PATCH 2/2 - The actual mailmap support.
Kousik Sanagavarapu (2):
  t/t6300: introduce test_bad_atom()
  ref-filter: add mailmap support
 Documentation/git-for-each-ref.txt |   6 +-
 ref-filter.c                       | 152 ++++++++++++++++++++++-------
 t/t6300-for-each-ref.sh            | 103 +++++++++++++++++++
 3 files changed, 225 insertions(+), 36 deletions(-)
-- 
2.42.0.160.g6905eb16ce.dirty
Kousik Sanagavarapu· Sep 20, 2023, 19:05 UTC · re: Kousik Sanagavarapu · lore

[PATCH 1/2] t/t6300: introduce test_bad_atom()

Introduce a new function "test_bad_atom()", which is similar to "test_atom()" but should be used to check whether the correct error message is shown on stderr.

Like "test_atom()", the new function takes three arguments. The three arguments specify the ref, the format and the expected error message respectively, with an optional fourth argument for tweaking "test_expect_*" (which is by default "success").

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
Show changes to t/t6300-for-each-ref.sh +20 −0
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..15b4622f57 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
 	test_must_fail git for-each-ref --format="%(objectname:short=foo)"
 '
 
+test_bad_atom() {
+	case "$1" in
+	head) ref=refs/heads/main ;;
+	 tag) ref=refs/tags/testtag ;;
+	 sym) ref=refs/heads/sym ;;
+	   *) ref=$1 ;;
+	esac
+	printf '%s\n' "$3">expect
+	test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
+		test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
+		test_cmp expect actual
+	"
+}
+
+test_bad_atom head 'authoremail:foo' \
+	'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+	'fatal: unrecognized %(taggeremail) argument:  trim'
+
 test_date () {
 	f=$1 &&
 	committer_date=$2 &&
-- 
2.42.0.160.g6905eb16ce.dirty
Junio C Hamano· Sep 20, 2023, 22:56 UTC · re: Kousik Sanagavarapu · lore

Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()

Kousik Sanagavarapu <five231003@gmail.com> writes:
Show 25 quoted lines
> Introduce a new function "test_bad_atom()", which is similar to
> "test_atom()" but should be used to check whether the correct error
> message is shown on stderr.
>
> Like "test_atom()", the new function takes three arguments. The three
> arguments specify the ref, the format and the expected error message
> respectively, with an optional fourth argument for tweaking
> "test_expect_*" (which is by default "success").
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Hariom Verma <hariom18599@gmail.com>
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
>  t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 7b943fd34c..15b4622f57 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
>  	test_must_fail git for-each-ref --format="%(objectname:short=foo)"
>  '
>  
> +test_bad_atom() {
Style: have SP on both sides of "()".
Show 6 quoted lines
> +	case "$1" in
> +	head) ref=refs/heads/main ;;
> +	 tag) ref=refs/tags/testtag ;;
> +	 sym) ref=refs/heads/sym ;;
> +	   *) ref=$1 ;;
> +	esac

Somehow this indirection makes the two examples we see below harder to understand. Why shouldn't we just write the full refname on th command line of test_bad_atom? That would make it crystal clear which ref each test works on. It does not help that both 'head' and 'sym' refer to a local branch (if the former referred to .git/HEAD or .git/remotes/origin/HEAD it may have been an excellent choice of the name, but that is not what is going on).

> +	printf '%s\n' "$3">expect
Style: need SP before (but not after) '>'.
> +	test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
> +		test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
> +		test_cmp expect actual
> +	"

It is error prone to have the executable part of test_expect_{success,failure} inside a pair of double quotes and have $variable interpolated _before_ even the arguments to test_expect_{success,failure} are formed. It is much more preferrable to write

	test_bad_atom () {
		ref=$1 format=$2
		printf '%s\n' "$3" >expect
		$test_do=test_expect_${4:-success}
		$test_do $PREREQ "err basic atom: $ref $format" '
			test_must_fail git for-each-ref \
				--format="%($format)" "$ref" 2>error &&
			test_cmp expect error
		'
	}

This is primarily because you cannot control what is in "$2" to ensure the correctness of the test we see above locally (i.e. if your caller has a single-quote in "$2", the shell script you create for running test_expect_{success,failure} would be syntactically incorrect). By enclosing the executable part inside a pair of single quotes, and having the $variables interpolated when that executable part is `eval`ed when test_expect_{success,failure} runs, you will avoid such problems, and those reading the above can locally know that you are aware of and correctly avoiding such problems.

I guess three among four problems I just pointed out you blindly copied from test_atom. But let's not spread badness (preliminary clean-up to existing badness would be welcome instead).

Show 7 quoted lines
> +}
> +
> +test_bad_atom head 'authoremail:foo' \
> +	'fatal: unrecognized %(authoremail) argument: foo'
> +
> +test_bad_atom tag 'taggeremail:localpart trim' \
> +	'fatal: unrecognized %(taggeremail) argument:  trim'

It is strange to see double SP before 'trim' in this error message. Are we etching a code mistake in stone here? Wouldn't the error message say "...argument: localpart trim" instead, perhaps?

>  test_date () {
>  	f=$1 &&
>  	committer_date=$2 &&
Junio C Hamano· Sep 20, 2023, 23:09 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()

Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
>> +	case "$1" in
>> +	head) ref=refs/heads/main ;;
>> +	 tag) ref=refs/tags/testtag ;;
>> +	 sym) ref=refs/heads/sym ;;
>> +	   *) ref=$1 ;;
>> +	esac
>
> Somehow this indirection makes the two examples we see below harder
> to understand.  ... It does not help that both 'head' and
> 'sym' refer to a local branch ...

Ah, this "sym" thing is a (rather unnatural) symbolic ref inside refs/heads/ hierarchy, so the naming makes halfway sense. As it is also used in test_atom, I no longer find it all that much disturbing.

Everything else I said in my review still stands, I would think.
Thanks.
Junio C Hamano· Sep 20, 2023, 23:21 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
>> +test_bad_atom tag 'taggeremail:localpart trim' \
>> +	'fatal: unrecognized %(taggeremail) argument:  trim'
>
> It is strange to see double SP before 'trim' in this error message.
> Are we etching a code mistake in stone here?  Wouldn't the error
> message say "...argument: localpart trim" instead, perhaps?

I tried. The fatal message does say ...argument: localpart trim" as I suspected, when you ask for 'taggeremail:localpart trim'.

I think I know what is going on. With the [PATCH 1/2] as-is, this piece does not pass. But because the error message from parsing gets broken by [PATCH 2/2], after applying [PATCH 2/2], the error message will become what the above test expects, hiding the new breakage in the code. And it probably was not noticed before you sent the patches, because you did not test [PATCH 1/2] alone.

Kousik Sanagavarapu· Sep 21, 2023, 18:57 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()

Sorry for the late reply.
On Wed, Sep 20, 2023 at 03:56:28PM -0700, Junio C Hamano wrote:
Show 35 quoted lines
> Kousik Sanagavarapu <five231003@gmail.com> writes:
> 
> > Introduce a new function "test_bad_atom()", which is similar to
> > "test_atom()" but should be used to check whether the correct error
> > message is shown on stderr.
> >
> > Like "test_atom()", the new function takes three arguments. The three
> > arguments specify the ref, the format and the expected error message
> > respectively, with an optional fourth argument for tweaking
> > "test_expect_*" (which is by default "success").
> >
> > Mentored-by: Christian Couder <christian.couder@gmail.com>
> > Mentored-by: Hariom Verma <hariom18599@gmail.com>
> > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> > ---
> >  t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> > index 7b943fd34c..15b4622f57 100755
> > --- a/t/t6300-for-each-ref.sh
> > +++ b/t/t6300-for-each-ref.sh
> > @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
> >  	test_must_fail git for-each-ref --format="%(objectname:short=foo)"
> >  '
> >  
> > +test_bad_atom() {
> 
> Style: have SP on both sides of "()".
> 
> [...]
>
> > +	printf '%s\n' "$3">expect
> 
> Style: need SP before (but not after) '>'.
I'll make these style changes, they slipped by.
Show 31 quoted lines
> > +	test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
> > +		test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
> > +		test_cmp expect actual
> > +	"
> 
> It is error prone to have the executable part of test_expect_{success,failure}
> inside a pair of double quotes and have $variable interpolated
> _before_ even the arguments to test_expect_{success,failure} are
> formed.  It is much more preferrable to write
> 
> 	test_bad_atom () {
> 		ref=$1 format=$2
> 		printf '%s\n' "$3" >expect
> 		$test_do=test_expect_${4:-success}
> 
> 		$test_do $PREREQ "err basic atom: $ref $format" '
> 			test_must_fail git for-each-ref \
> 				--format="%($format)" "$ref" 2>error &&
> 			test_cmp expect error
> 		'
> 	}
> 
> This is primarily because you cannot control what is in "$2" to
> ensure the correctness of the test we see above locally (i.e. if
> your caller has a single-quote in "$2", the shell script you create
> for running test_expect_{success,failure} would be syntactically
> incorrect).  By enclosing the executable part inside a pair of
> single quotes, and having the $variables interpolated when that
> executable part is `eval`ed when test_expect_{success,failure} runs,
> you will avoid such problems, and those reading the above can locally
> know that you are aware of and correctly avoiding such problems.
I see.
> I guess three among four problems I just pointed out you blindly
> copied from test_atom.  But let's not spread badness (preliminary
> clean-up to existing badness would be welcome instead).

Yeah, I had copied it from test_atom. Although I didn't realize that it was bad to implement test_bad_atom the way I did. Thanks for such a nice explanation. So I guess we can include the test_atom cleanup in this series?

Show 15 quoted lines
> > +}
> > +
> > +test_bad_atom head 'authoremail:foo' \
> > +	'fatal: unrecognized %(authoremail) argument: foo'
> > +
> > +test_bad_atom tag 'taggeremail:localpart trim' \
> > +	'fatal: unrecognized %(taggeremail) argument:  trim'
> 
> It is strange to see double SP before 'trim' in this error message.
> Are we etching a code mistake in stone here?  Wouldn't the error
> message say "...argument: localpart trim" instead, perhaps?
> 
> >  test_date () {
> >  	f=$1 &&
> >  	committer_date=$2 &&

So I read the the other replies and it seems that it indeed hides a breakage and yeah I hadn't tested PATCH 1/2 independently. I'll change this too.

Thanks
Kousik Sanagavarapu· Sep 20, 2023, 19:05 UTC · re: Kousik Sanagavarapu · lore

[PATCH 2/2] ref-filter: add mailmap support

Add mailmap support to ref-filter formats which are similar in pretty. This support is such that the following pretty placeholders are equivalent to the new ref-filter atoms:

	%aN = authorname:mailmap
	%cN = committername:mailmap
	%aE = authoremail:mailmap
	%aL = authoremail:mailmap,localpart
	%cE = committeremail:mailmap
	%cL = committeremail:mailmap,localpart

Additionally, mailmap can also be used with ":trim" option for email by doing something like "authoremail:mailmap,trim".

The above also applies for the "tagger" atom, that is, "taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim" and "taggername:mailmap,localpart".

The functionality of ":trim" and ":localpart" remains the same. That is, ":trim" gives the email, but without the angle brackets and ":localpart" gives the part of the email before the '@' character (if such a character is not found then we directly grab everything between the angle brackets).

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |   6 +-
 ref-filter.c                       | 152 ++++++++++++++++++++++-------
 t/t6300-for-each-ref.sh            |  83 ++++++++++++++++
 3 files changed, 205 insertions(+), 36 deletions(-)
Show changes to 3 files +205 −36

Documentation/git-for-each-ref.txt, ref-filter.c, t/t6300-for-each-ref.sh

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
 and `date` to extract the named component.  For email fields (`authoremail`,
 `committeremail` and `taggeremail`), `:trim` can be appended to get the email
 without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
 
 The raw data in an object is `raw`.
 
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
 #include "oid-array.h"
 #include "repository.h"
 #include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
 #include "remote.h"
 #include "color.h"
 #include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
 		struct {
 			enum { O_SIZE, O_SIZE_DISK } option;
 		} objectsize;
-		struct email_option {
-			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+		struct {
+			enum { N_RAW, N_MAILMAP } option;
+		} name_option;
+		struct {
+			enum {
+				EO_RAW = 0,
+				EO_TRIM = 1<<0,
+				EO_LOCALPART = 1<<1,
+				EO_MAILMAP = 1<<2,
+			} option;
 		} email_option;
 		struct {
 			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int person_email_atom_parser(struct ref_format *format UNUSED,
-				    struct used_atom *atom,
-				    const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+				   struct used_atom *atom,
+				   const char *arg, struct strbuf *err)
 {
 	if (!arg)
-		atom->u.email_option.option = EO_RAW;
-	else if (!strcmp(arg, "trim"))
-		atom->u.email_option.option = EO_TRIM;
-	else if (!strcmp(arg, "localpart"))
-		atom->u.email_option.option = EO_LOCALPART;
+		atom->u.name_option.option = N_RAW;
+	else if (!strcmp(arg, "mailmap"))
+		atom->u.name_option.option = N_MAILMAP;
 	else
 		return err_bad_arg(err, atom->name, arg);
 	return 0;
 }
 
+static int email_atom_option_parser(struct used_atom *atom,
+				    const char **arg, struct strbuf *err)
+{
+	if (!*arg)
+		return EO_RAW;
+	if (skip_prefix(*arg, "trim", arg))
+		return EO_TRIM;
+	if (skip_prefix(*arg, "localpart", arg))
+		return EO_LOCALPART;
+	if (skip_prefix(*arg, "mailmap", arg))
+		return EO_MAILMAP;
+	return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+				    struct used_atom *atom,
+				    const char *arg, struct strbuf *err)
+{
+	for (;;) {
+		int opt = email_atom_option_parser(atom, &arg, err);
+		const char *bad_arg = arg;
+
+		if (opt < 0)
+			return err_bad_arg(err, atom->name, bad_arg);
+		atom->u.email_option.option |= opt;
+
+		if (!arg || !*arg)
+			break;
+		if (*arg == ',')
+			arg++;
+		else
+			return err_bad_arg(err, atom->name, bad_arg);
+	}
+	return 0;
+}
+
 static int refname_atom_parser(struct ref_format *format UNUSED,
 			       struct used_atom *atom,
 			       const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
 	[ATOM_TYPE] = { "type", SOURCE_OBJ },
 	[ATOM_TAG] = { "tag", SOURCE_OBJ },
 	[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
-	[ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+	[ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
-	[ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+	[ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
-	[ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+	[ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
 	return xstrdup("");
 }
 
+static const char *find_end_of_email(const char *email, int opt)
+{
+	const char *eoemail;
+
+	if (opt & EO_LOCALPART) {
+		eoemail = strchr(email, '@');
+		if (eoemail)
+			return eoemail;
+		return strchr(email, '>');
+	}
+
+	if (opt & EO_TRIM)
+		return strchr(email, '>');
+
+	/*
+	 * The option here is either the raw email option or the raw
+	 * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+	 * we directly grab the whole email including the closing
+	 * angle brackets.
+	 *
+	 * If EO_MAILMAP was set with any other option (that is either
+	 * EO_TRIM or EO_LOCALPART), we already grab the end of email
+	 * above.
+	 */
+	eoemail = strchr(email, '>');
+	if (eoemail)
+		eoemail++;
+	return eoemail;
+}
+
 static const char *copy_email(const char *buf, struct used_atom *atom)
 {
 	const char *email = strchr(buf, '<');
 	const char *eoemail;
+	int opt = atom->u.email_option.option;
+
 	if (!email)
 		return xstrdup("");
-	switch (atom->u.email_option.option) {
-	case EO_RAW:
-		eoemail = strchr(email, '>');
-		if (eoemail)
-			eoemail++;
-		break;
-	case EO_TRIM:
-		email++;
-		eoemail = strchr(email, '>');
-		break;
-	case EO_LOCALPART:
+
+	if (opt & (EO_LOCALPART | EO_TRIM))
 		email++;
-		eoemail = strchr(email, '@');
-		if (!eoemail)
-			eoemail = strchr(email, '>');
-		break;
-	default:
-		BUG("unknown email option");
-	}
 
+	eoemail = find_end_of_email(email, opt);
 	if (!eoemail)
 		return xstrdup("");
 	return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	v->value = 0;
 }
 
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
 /* See grab_values */
 static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
 {
 	int i;
 	int wholen = strlen(who);
 	const char *wholine = NULL;
+	const char *headers[] = { "author ", "committer ",
+				  "tagger ", NULL };
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
 		struct atom_value *v = &val[i];
+		struct strbuf mailmap_buf = STRBUF_INIT;
+
 		if (!!deref != (*name == '*'))
 			continue;
 		if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 		if (strncmp(who, name, wholen))
 			continue;
 		if (name[wholen] != 0 &&
-		    strcmp(name + wholen, "name") &&
+		    !starts_with(name + wholen, "name") &&
 		    !starts_with(name + wholen, "email") &&
 		    !starts_with(name + wholen, "date"))
 			continue;
-		if (!wholine)
+
+		if ((starts_with(name + wholen, "name") &&
+		    (atom->u.name_option.option == N_MAILMAP)) ||
+		    (starts_with(name + wholen, "email") &&
+		    (atom->u.email_option.option & EO_MAILMAP))) {
+			if (!mailmap.items)
+				read_mailmap(&mailmap);
+			strbuf_addstr(&mailmap_buf, buf);
+			apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+			wholine = find_wholine(who, wholen, mailmap_buf.buf);
+		} else {
 			wholine = find_wholine(who, wholen, buf);
+		}
+
 		if (!wholine)
 			return; /* no point looking for it */
 		if (name[wholen] == 0)
 			v->s = copy_line(wholine);
-		else if (!strcmp(name + wholen, "name"))
+		else if (starts_with(name + wholen, "name"))
 			v->s = copy_name(wholine);
 		else if (starts_with(name + wholen, "email"))
 			v->s = copy_email(wholine, &used_atom[i]);
 		else if (starts_with(name + wholen, "date"))
 			grab_date(wholine, v, name);
+
+		strbuf_release(&mailmap_buf);
 	}
 
 	/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 15b4622f57..1fb464ca50 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
 	disklen sha1:138
 	disklen sha256:154
 	EOF
+
+	# setup .mailmap
+	cat >.mailmap <<-EOF &&
+	A Thor <athor@example.com> A U Thor <author@example.com>
+	C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+	EOF
+
 	setdate_and_increment &&
 	echo "Using $datestamp" > one &&
 	git add one &&
@@ -141,15 +148,31 @@ test_atom head '*objectname' ''
 test_atom head '*objecttype' ''
 test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
 test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
 test_atom head authoremail '<author@example.com>'
 test_atom head authoremail:trim 'author@example.com'
 test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
 test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
 test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
 test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
 test_atom head committeremail '<committer@example.com>'
 test_atom head committeremail:trim 'committer@example.com'
 test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
 test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
 test_atom head tag ''
 test_atom head tagger ''
@@ -199,22 +222,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
 test_atom tag '*objecttype' 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
 test_atom tag authoremail ''
 test_atom tag authoremail:trim ''
 test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
 test_atom tag authordate ''
 test_atom tag committer ''
 test_atom tag committername ''
+test_atom tag committername:mailmap ''
 test_atom tag committeremail ''
 test_atom tag committeremail:trim ''
 test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
 test_atom tag committerdate ''
 test_atom tag tag 'testtag'
 test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
 test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
 test_atom tag taggeremail '<committer@example.com>'
 test_atom tag taggeremail:trim 'committer@example.com'
 test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
 test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
 test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
 test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -284,9 +331,45 @@ test_bad_atom() {
 test_bad_atom head 'authoremail:foo' \
 	'fatal: unrecognized %(authoremail) argument: foo'
 
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+	'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+	'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+	'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+	'fatal: unrecognized %(committeremail) argument:  '
+
+test_bad_atom head 'committeremail: trim,foo' \
+	'fatal: unrecognized %(committeremail) argument:  trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+	'fatal: unrecognized %(committeremail) argument:  '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+	'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+	'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+	'fatal: unrecognized %(taggeremail) argument:  foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+	'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+	'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
 test_bad_atom tag 'taggeremail:localpart trim' \
 	'fatal: unrecognized %(taggeremail) argument:  trim'
 
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+	'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
+
 test_date () {
 	f=$1 &&
 	committer_date=$2 &&
-- 
2.42.0.160.g6905eb16ce.dirty
Kousik Sanagavarapu· Sep 25, 2023, 17:43 UTC · re: Kousik Sanagavarapu · lore

[PATCH v2 0/3] Add mailmap support to ref-filter

Thanks Junio for review on the previous round.
PATCH 1/3 - Cleanup test_atom to be less error-prone
PATCH 2/3 - Fix hidden breakage
PATCH 3/3 - Unchanged
Kousik Sanagavarapu (3):
  t/t6300: cleanup test_atom
  t/t6300: introduce test_bad_atom
  ref-filter: add mailmap support
 Documentation/git-for-each-ref.txt |   6 +-
 ref-filter.c                       | 152 ++++++++++++++++++++++-------
 t/t6300-for-each-ref.sh            | 123 +++++++++++++++++++++--
 3 files changed, 239 insertions(+), 42 deletions(-)
Range-diff against v1:
-:  ---------- > 1:  b28e858e35 t/t6300: cleanup test_atom
1:  0a90b67889 ! 2:  ee90d017d5 t/t6300: introduce test_bad_atom()
    @@ Metadata
     Author: Kousik Sanagavarapu <five231003@gmail.com>
      ## Commit message ##
    -    t/t6300: introduce test_bad_atom()
    +    t/t6300: introduce test_bad_atom
    -    Introduce a new function "test_bad_atom()", which is similar to
    +    Introduce a new function "test_bad_atom", which is similar to
         "test_atom()" but should be used to check whether the correct error
         message is shown on stderr.
    -    Like "test_atom()", the new function takes three arguments. The three
    +    Like "test_atom", the new function takes three arguments. The three
         arguments specify the ref, the format and the expected error message
         respectively, with an optional fourth argument for tweaking
         "test_expect_*" (which is by default "success").
    @@ t/t6300-for-each-ref.sh: test_expect_success 'arguments to %(objectname:short=)
        test_must_fail git for-each-ref --format="%(objectname:short=foo)"
      '
    -+test_bad_atom() {
    ++test_bad_atom () {
     +  case "$1" in
     +  head) ref=refs/heads/main ;;
     +   tag) ref=refs/tags/testtag ;;
     +   sym) ref=refs/heads/sym ;;
     +     *) ref=$1 ;;
     +  esac
    -+  printf '%s\n' "$3">expect
    -+  test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
    -+          test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
    -+          test_cmp expect actual
    -+  "
    ++  format=$2
    ++  test_do=test_expect_${4:-success}
    ++
    ++  printf '%s\n' "$3" >expect
    ++  $test_do $PREREQ "err basic atom: $ref $format" '
    ++          test_must_fail git for-each-ref \
    ++                  --format="%($format)" "$ref" 2>error &&
    ++          test_cmp expect error
    ++  '
     +}
     +
     +test_bad_atom head 'authoremail:foo' \
     +  'fatal: unrecognized %(authoremail) argument: foo'
     +
     +test_bad_atom tag 'taggeremail:localpart trim' \
    -+  'fatal: unrecognized %(taggeremail) argument:  trim'
    ++  'fatal: unrecognized %(taggeremail) argument: localpart trim'
     +
      test_date () {
        f=$1 &&
2:  63fc69f4dc ! 3:  fdc14fe80b ref-filter: add mailmap support
    @@ t/t6300-for-each-ref.sh: test_atom tag '*objectname' $(git rev-parse refs/tags/t
      test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
      test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
      test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
    -@@ t/t6300-for-each-ref.sh: test_bad_atom() {
    +@@ t/t6300-for-each-ref.sh: test_bad_atom () {
      test_bad_atom head 'authoremail:foo' \
        'fatal: unrecognized %(authoremail) argument: foo'
    @@ t/t6300-for-each-ref.sh: test_bad_atom() {
     +  'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
     +
      test_bad_atom tag 'taggeremail:localpart trim' \
    -   'fatal: unrecognized %(taggeremail) argument:  trim'
    -
    +-  'fatal: unrecognized %(taggeremail) argument: localpart trim'
    ++  'fatal: unrecognized %(taggeremail) argument:  trim'
    ++
     +test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
     +  'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
    -+
    +
      test_date () {
        f=$1 &&
    -   committer_date=$2 &&
-- 
2.42.0.273.ge948a9aaf4
Kousik Sanagavarapu· Sep 25, 2023, 17:43 UTC · re: Kousik Sanagavarapu · lore

[PATCH v2 1/3] t/t6300: cleanup test_atom

Previously, when the executable part of "test_expect_{success,failure}" (inside "test_atom") got "eval"ed, it would have been syntactically incorrect if the second argument ($2, which is the format) to "test_atom" were enclosed in single quotes because the $variables would get interpolated even before the arguments to "test_expect_{success,failure}" are formed.

So fix this and also some style issues along the way.
Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/t6300-for-each-ref.sh | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
Show changes to t/t6300-for-each-ref.sh +10 −6
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..7ba9949376 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -41,25 +41,29 @@ test_expect_success setup '
 	git config push.default current
 '
 
-test_atom() {
+test_atom () {
 	case "$1" in
 		head) ref=refs/heads/main ;;
 		 tag) ref=refs/tags/testtag ;;
 		 sym) ref=refs/heads/sym ;;
 		   *) ref=$1 ;;
 	esac
+	format=$2
+	test_do=test_expect_${4:-success}
+
 	printf '%s\n' "$3" >expected
-	test_expect_${4:-success} $PREREQ "basic atom: $1 $2" "
-		git for-each-ref --format='%($2)' $ref >actual &&
+	$test_do $PREREQ "basic atom: $ref $format" '
+		git for-each-ref --format="%($format)" "$ref" >actual &&
 		sanitize_pgp <actual >actual.clean &&
 		test_cmp expected actual.clean
-	"
+	'
+
 	# Automatically test "contents:size" atom after testing "contents"
-	if test "$2" = "contents"
+	if test "$format" = "contents"
 	then
 		# for commit leg, $3 is changed there
 		expect=$(printf '%s' "$3" | wc -c)
-		test_expect_${4:-success} $PREREQ "basic atom: $1 contents:size" '
+		$test_do $PREREQ "basic atom: $ref contents:size" '
 			type=$(git cat-file -t "$ref") &&
 			case $type in
 			tag)
-- 
2.42.0.273.ge948a9aaf4
Kousik Sanagavarapu· Sep 25, 2023, 17:43 UTC · re: Kousik Sanagavarapu · lore

[PATCH v2 2/3] t/t6300: introduce test_bad_atom

Introduce a new function "test_bad_atom", which is similar to "test_atom()" but should be used to check whether the correct error message is shown on stderr.

Like "test_atom", the new function takes three arguments. The three arguments specify the ref, the format and the expected error message respectively, with an optional fourth argument for tweaking "test_expect_*" (which is by default "success").

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/t6300-for-each-ref.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
Show changes to t/t6300-for-each-ref.sh +24 −0
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7ba9949376..e4ec2926d6 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -271,6 +271,30 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
 	test_must_fail git for-each-ref --format="%(objectname:short=foo)"
 '
 
+test_bad_atom () {
+	case "$1" in
+	head) ref=refs/heads/main ;;
+	 tag) ref=refs/tags/testtag ;;
+	 sym) ref=refs/heads/sym ;;
+	   *) ref=$1 ;;
+	esac
+	format=$2
+	test_do=test_expect_${4:-success}
+
+	printf '%s\n' "$3" >expect
+	$test_do $PREREQ "err basic atom: $ref $format" '
+		test_must_fail git for-each-ref \
+			--format="%($format)" "$ref" 2>error &&
+		test_cmp expect error
+	'
+}
+
+test_bad_atom head 'authoremail:foo' \
+	'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+	'fatal: unrecognized %(taggeremail) argument: localpart trim'
+
 test_date () {
 	f=$1 &&
 	committer_date=$2 &&
-- 
2.42.0.273.ge948a9aaf4
Kousik Sanagavarapu· Sep 25, 2023, 17:43 UTC · re: Kousik Sanagavarapu · lore

[PATCH v2 3/3] ref-filter: add mailmap support

Add mailmap support to ref-filter formats which are similar in pretty. This support is such that the following pretty placeholders are equivalent to the new ref-filter atoms:

	%aN = authorname:mailmap
	%cN = committername:mailmap
	%aE = authoremail:mailmap
	%aL = authoremail:mailmap,localpart
	%cE = committeremail:mailmap
	%cL = committeremail:mailmap,localpart

Additionally, mailmap can also be used with ":trim" option for email by doing something like "authoremail:mailmap,trim".

The above also applies for the "tagger" atom, that is, "taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim" and "taggername:mailmap,localpart".

The functionality of ":trim" and ":localpart" remains the same. That is, ":trim" gives the email, but without the angle brackets and ":localpart" gives the part of the email before the '@' character (if such a character is not found then we directly grab everything between the angle brackets).

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |   6 +-
 ref-filter.c                       | 152 ++++++++++++++++++++++-------
 t/t6300-for-each-ref.sh            |  85 +++++++++++++++-
 3 files changed, 206 insertions(+), 37 deletions(-)
Show changes to 3 files +206 −37

Documentation/git-for-each-ref.txt, ref-filter.c, t/t6300-for-each-ref.sh

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
 and `date` to extract the named component.  For email fields (`authoremail`,
 `committeremail` and `taggeremail`), `:trim` can be appended to get the email
 without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
 
 The raw data in an object is `raw`.
 
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
 #include "oid-array.h"
 #include "repository.h"
 #include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
 #include "remote.h"
 #include "color.h"
 #include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
 		struct {
 			enum { O_SIZE, O_SIZE_DISK } option;
 		} objectsize;
-		struct email_option {
-			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+		struct {
+			enum { N_RAW, N_MAILMAP } option;
+		} name_option;
+		struct {
+			enum {
+				EO_RAW = 0,
+				EO_TRIM = 1<<0,
+				EO_LOCALPART = 1<<1,
+				EO_MAILMAP = 1<<2,
+			} option;
 		} email_option;
 		struct {
 			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int person_email_atom_parser(struct ref_format *format UNUSED,
-				    struct used_atom *atom,
-				    const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+				   struct used_atom *atom,
+				   const char *arg, struct strbuf *err)
 {
 	if (!arg)
-		atom->u.email_option.option = EO_RAW;
-	else if (!strcmp(arg, "trim"))
-		atom->u.email_option.option = EO_TRIM;
-	else if (!strcmp(arg, "localpart"))
-		atom->u.email_option.option = EO_LOCALPART;
+		atom->u.name_option.option = N_RAW;
+	else if (!strcmp(arg, "mailmap"))
+		atom->u.name_option.option = N_MAILMAP;
 	else
 		return err_bad_arg(err, atom->name, arg);
 	return 0;
 }
 
+static int email_atom_option_parser(struct used_atom *atom,
+				    const char **arg, struct strbuf *err)
+{
+	if (!*arg)
+		return EO_RAW;
+	if (skip_prefix(*arg, "trim", arg))
+		return EO_TRIM;
+	if (skip_prefix(*arg, "localpart", arg))
+		return EO_LOCALPART;
+	if (skip_prefix(*arg, "mailmap", arg))
+		return EO_MAILMAP;
+	return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+				    struct used_atom *atom,
+				    const char *arg, struct strbuf *err)
+{
+	for (;;) {
+		int opt = email_atom_option_parser(atom, &arg, err);
+		const char *bad_arg = arg;
+
+		if (opt < 0)
+			return err_bad_arg(err, atom->name, bad_arg);
+		atom->u.email_option.option |= opt;
+
+		if (!arg || !*arg)
+			break;
+		if (*arg == ',')
+			arg++;
+		else
+			return err_bad_arg(err, atom->name, bad_arg);
+	}
+	return 0;
+}
+
 static int refname_atom_parser(struct ref_format *format UNUSED,
 			       struct used_atom *atom,
 			       const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
 	[ATOM_TYPE] = { "type", SOURCE_OBJ },
 	[ATOM_TAG] = { "tag", SOURCE_OBJ },
 	[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
-	[ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+	[ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
-	[ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+	[ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
-	[ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+	[ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
 	[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
 	[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
 	[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
 	return xstrdup("");
 }
 
+static const char *find_end_of_email(const char *email, int opt)
+{
+	const char *eoemail;
+
+	if (opt & EO_LOCALPART) {
+		eoemail = strchr(email, '@');
+		if (eoemail)
+			return eoemail;
+		return strchr(email, '>');
+	}
+
+	if (opt & EO_TRIM)
+		return strchr(email, '>');
+
+	/*
+	 * The option here is either the raw email option or the raw
+	 * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+	 * we directly grab the whole email including the closing
+	 * angle brackets.
+	 *
+	 * If EO_MAILMAP was set with any other option (that is either
+	 * EO_TRIM or EO_LOCALPART), we already grab the end of email
+	 * above.
+	 */
+	eoemail = strchr(email, '>');
+	if (eoemail)
+		eoemail++;
+	return eoemail;
+}
+
 static const char *copy_email(const char *buf, struct used_atom *atom)
 {
 	const char *email = strchr(buf, '<');
 	const char *eoemail;
+	int opt = atom->u.email_option.option;
+
 	if (!email)
 		return xstrdup("");
-	switch (atom->u.email_option.option) {
-	case EO_RAW:
-		eoemail = strchr(email, '>');
-		if (eoemail)
-			eoemail++;
-		break;
-	case EO_TRIM:
-		email++;
-		eoemail = strchr(email, '>');
-		break;
-	case EO_LOCALPART:
+
+	if (opt & (EO_LOCALPART | EO_TRIM))
 		email++;
-		eoemail = strchr(email, '@');
-		if (!eoemail)
-			eoemail = strchr(email, '>');
-		break;
-	default:
-		BUG("unknown email option");
-	}
 
+	eoemail = find_end_of_email(email, opt);
 	if (!eoemail)
 		return xstrdup("");
 	return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	v->value = 0;
 }
 
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
 /* See grab_values */
 static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
 {
 	int i;
 	int wholen = strlen(who);
 	const char *wholine = NULL;
+	const char *headers[] = { "author ", "committer ",
+				  "tagger ", NULL };
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
 		struct atom_value *v = &val[i];
+		struct strbuf mailmap_buf = STRBUF_INIT;
+
 		if (!!deref != (*name == '*'))
 			continue;
 		if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 		if (strncmp(who, name, wholen))
 			continue;
 		if (name[wholen] != 0 &&
-		    strcmp(name + wholen, "name") &&
+		    !starts_with(name + wholen, "name") &&
 		    !starts_with(name + wholen, "email") &&
 		    !starts_with(name + wholen, "date"))
 			continue;
-		if (!wholine)
+
+		if ((starts_with(name + wholen, "name") &&
+		    (atom->u.name_option.option == N_MAILMAP)) ||
+		    (starts_with(name + wholen, "email") &&
+		    (atom->u.email_option.option & EO_MAILMAP))) {
+			if (!mailmap.items)
+				read_mailmap(&mailmap);
+			strbuf_addstr(&mailmap_buf, buf);
+			apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+			wholine = find_wholine(who, wholen, mailmap_buf.buf);
+		} else {
 			wholine = find_wholine(who, wholen, buf);
+		}
+
 		if (!wholine)
 			return; /* no point looking for it */
 		if (name[wholen] == 0)
 			v->s = copy_line(wholine);
-		else if (!strcmp(name + wholen, "name"))
+		else if (starts_with(name + wholen, "name"))
 			v->s = copy_name(wholine);
 		else if (starts_with(name + wholen, "email"))
 			v->s = copy_email(wholine, &used_atom[i]);
 		else if (starts_with(name + wholen, "date"))
 			grab_date(wholine, v, name);
+
+		strbuf_release(&mailmap_buf);
 	}
 
 	/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index e4ec2926d6..00a060df0b 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
 	disklen sha1:138
 	disklen sha256:154
 	EOF
+
+	# setup .mailmap
+	cat >.mailmap <<-EOF &&
+	A Thor <athor@example.com> A U Thor <author@example.com>
+	C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+	EOF
+
 	setdate_and_increment &&
 	echo "Using $datestamp" > one &&
 	git add one &&
@@ -145,15 +152,31 @@ test_atom head '*objectname' ''
 test_atom head '*objecttype' ''
 test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
 test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
 test_atom head authoremail '<author@example.com>'
 test_atom head authoremail:trim 'author@example.com'
 test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
 test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
 test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
 test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
 test_atom head committeremail '<committer@example.com>'
 test_atom head committeremail:trim 'committer@example.com'
 test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
 test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
 test_atom head tag ''
 test_atom head tagger ''
@@ -203,22 +226,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
 test_atom tag '*objecttype' 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
 test_atom tag authoremail ''
 test_atom tag authoremail:trim ''
 test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
 test_atom tag authordate ''
 test_atom tag committer ''
 test_atom tag committername ''
+test_atom tag committername:mailmap ''
 test_atom tag committeremail ''
 test_atom tag committeremail:trim ''
 test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
 test_atom tag committerdate ''
 test_atom tag tag 'testtag'
 test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
 test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
 test_atom tag taggeremail '<committer@example.com>'
 test_atom tag taggeremail:trim 'committer@example.com'
 test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
 test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
 test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
 test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -292,8 +339,44 @@ test_bad_atom () {
 test_bad_atom head 'authoremail:foo' \
 	'fatal: unrecognized %(authoremail) argument: foo'
 
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+	'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+	'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+	'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+	'fatal: unrecognized %(committeremail) argument:  '
+
+test_bad_atom head 'committeremail: trim,foo' \
+	'fatal: unrecognized %(committeremail) argument:  trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+	'fatal: unrecognized %(committeremail) argument:  '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+	'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+	'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+	'fatal: unrecognized %(taggeremail) argument:  foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+	'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+	'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
 test_bad_atom tag 'taggeremail:localpart trim' \
-	'fatal: unrecognized %(taggeremail) argument: localpart trim'
+	'fatal: unrecognized %(taggeremail) argument:  trim'
+
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+	'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
 
 test_date () {
 	f=$1 &&
-- 
2.42.0.273.ge948a9aaf4

← back to recent threads