shortlog: pass the mailmap into the revision walker

From: CB Bailey <[email protected]>

shortlog always respects the mailmap in its output. Pass the mailmap into the revision walker to allow the mailmap to be used with revision limiting options such as '--author'.

This removes some apparently inconsistent behaviors when using '--author', such as not finding some or all commits for a given author which do appear under that author in an unrestricted invocation of shortlog or commits being summarized under a different author than the specified author.

---
 builtin/shortlog.c |  2 ++
 t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

index 88f88e97b2..a6fb00ade8 100644
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
 {
 	struct commit *commit;
 
+	rev->mailmap = &log->mailmap;
+
 	if (prepare_revision_walk(rev))
 		die(_("revision walk setup failed"));
 	while ((commit = get_revision(rev)) != NULL)

index 43b1522ea2..9bee35b06c 100755
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
 
 '
 
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+	printf "     1\tA U Thor <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="A U Thor" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     1\tCTO <[email protected]>\n" >expect &&
+
+	git shortlog -es --author=CTO HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     2\tOther Author <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Other Author" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     2\tSanta Claus <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Santa Claus" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     1\tSome Dude <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Some Dude" HEAD >actual &&
+	test_cmp expect actual
+'
+
 # git log with --pretty format which uses the name and email mailmap placemarkers
 cat >expect <<\EOF
 Author CTO <[email protected]> maps to CTO <[email protected]>

2.17.0.rc0

From: CB Bailey <[email protected]>

shortlog always respects the mailmap in its output. Pass the mailmap into the revision walker to allow the mailmap to be used with revision limiting options such as '--author'.

This removes some apparently inconsistent behaviors when using '--author', such as not finding some or all commits for a given author which do appear under that author in an unrestricted invocation of shortlog or commits being summarized under a different author than the specified author.

---

Resending with omitted s-o-b.

 builtin/shortlog.c |  2 ++
 t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

index 88f88e97b2..a6fb00ade8 100644
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
 {
 	struct commit *commit;
 
+	rev->mailmap = &log->mailmap;
+
 	if (prepare_revision_walk(rev))
 		die(_("revision walk setup failed"));
 	while ((commit = get_revision(rev)) != NULL)

index 43b1522ea2..9bee35b06c 100755
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
 
 '
 
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+	printf "     1\tA U Thor <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="A U Thor" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     1\tCTO <[email protected]>\n" >expect &&
+
+	git shortlog -es --author=CTO HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     2\tOther Author <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Other Author" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     2\tSanta Claus <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Santa Claus" HEAD >actual &&
+	test_cmp expect actual &&
+
+	printf "     1\tSome Dude <[email protected]>\n" >expect &&
+
+	git shortlog -es --author="Some Dude" HEAD >actual &&
+	test_cmp expect actual
+'
+
 # git log with --pretty format which uses the name and email mailmap placemarkers
 cat >expect <<\EOF
 Author CTO <[email protected]> maps to CTO <[email protected]>

2.17.0.rc0

I am a bit torn on this.

If an author of interest has entries in the mailmap, with the same name but three or more different e-mail addresses, you could do

    $ git shortlog [email protected] [email protected]

to find commits by the author under these two addresses, and exclude commits by the same author made under other addresses. With this patch applied, it becomes no longer possible, and you can only look for commits written under all of my identities, or none of them, which is a minor annoyance.

But more importantly, the user now needs to know which one is the "canonical" spelling of the author's ident [*1*]. Asking for commits written by [email protected] will not yield any result with the patch applied, no?

For the author in the above example, luckily, AUTHOR_NAME is the same and unique, so without your patch, you could still do

    $ git shortlog --author="Junio C"

to grab all of them, but there may be authors for whom there is no catch-all substring that would match all of the idents they ever used but still unique enough to match only that author, and I do agree that the new feature proposed by the patch in question would have uses in such a case.

Which nudges me to say that there needs a way to ask for the original behaviour, disabling the rewriting of commit author identity before --author filter kicks in.

There may also need a way to ask omitting mailmap processing even at the output phase. I do not think it is a particularly interesting feature to be able to ask for my commits under only two of my identities [*2*], but as long as that "feature" exists, it also should be possible to see the result of that "feature" more clearly by not merging them into a single list. "shortlog --no-use-mailmap" may be a way to do so, but unlike "log", the command currently does not take the "--[no-]use-mailmap" option.

[Footnote]

*1* Passing the query string given as --author=... through the
mailmap machinery _might_ be a possible ingredient to solve "the user needs to know the caonical spelling of the author ident" issue this patch has, but I do not think it generally is possible (e.g. how would you rewrite --author='.*@kernel.org').

*2* In other words, I think the loss of "find only my @kernel.org
and @twinsun.com commits" is mere annoyance and people can live with it. But I think "you must look for [email protected], and looking for [email protected] will not find my commits made under that identity" is a show stopper. That is why I use a very weak "may also need" to this "optionally not using mailmap at all" feature.