{"thread":{"id":"34750","subject":"[PATCH] git-commit: search author pattern against mailmap","startedAt":"2013-08-23T13:48:31Z","lastAt":"2013-08-26T21:38:31Z","messageCount":17,"participants":["Antoine Pelisse","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225719","messageId":"1377265711-11492-1-git-send-email-apelisse@gmail.com","threadId":"34750","inReplyTo":null,"subject":"[PATCH] git-commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-23T13:48:31Z","receivedAt":"2013-08-23T13:48:31Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"When committing for someone else, using the --author option, it can be\nnice to use the mailmap file to find the correct name spelling and email\naddress.\n\nCurrently, you would have to find the correct mapping in mailmap file\nfirst, and then use the full ident form when committing.\n\nLet's allow git-commit to find if an entry exists in mailmap file for\nthat pattern, and use that instead.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nHi,\nI would use that feature at work where I happen to commit some work for\nother colleagues, while we heavily rely on mailmap file to have decent indents.\n\nOn the other hand, I'm kind of embarrassed to add this new option to\ngit-commit.\n\n Documentation/git-commit.txt |  6 +++++-\n builtin/commit.c             | 16 +++++++++++++++-\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 1a7616c..9e3fe04 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n \t   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]\n \t   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\n \t   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\n-\t   [--date=<date>] [--cleanup=<mode>] [--[no-]status]\n+\t   [--use-mailmap] [--date=<date>] [--cleanup=<mode>] [--[no-]status]\n \t   [-i | -o] [-S[<keyid>]] [--] [<file>...]\n\n DESCRIPTION\n@@ -131,6 +131,10 @@ OPTIONS\n \tcommit by that author (i.e. rev-list --all -i --author=<author>);\n \tthe commit author is then copied from the first such commit found.\n\n+--use-mailmap::\n+\tWhen used with `--author=<author>`, match the <author> pattern\n+\tagainst mapped name and email. See linkgit:git-shortlog[1].\n+\n --date=<date>::\n \tOverride the author date used in the commit.\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 10acc53..fbd0664 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -30,6 +30,7 @@\n #include \"column.h\"\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n+#include \"mailmap.h\"\n\n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <pathspec>...\"),\n@@ -87,6 +88,7 @@ static enum {\n } commit_style;\n\n static const char *logfile, *force_author;\n+static int mailmap;\n static const char *template_file;\n /*\n  * The _message variables are commit names from which to take\n@@ -945,13 +947,24 @@ static const char *find_author_by_nickname(const char *name)\n \tav[++ac] = buf.buf;\n \tav[++ac] = NULL;\n \tsetup_revisions(ac, av, &revs, NULL);\n+\tif (mailmap) {\n+\t\trevs.mailmap = xcalloc(1, sizeof(struct string_list));\n+\t\tread_mailmap(revs.mailmap, NULL);\n+\t}\n \tprepare_revision_walk(&revs);\n \tcommit = get_revision(&revs);\n \tif (commit) {\n \t\tstruct pretty_print_context ctx = {0};\n+\t\tconst char *format;\n+\n+\t\tif (mailmap)\n+\t\t\tformat = \"%aN <%aE>\";\n+\t\telse\n+\t\t\tformat = \"%an <%ae>\";\n+\n \t\tctx.date_mode = DATE_NORMAL;\n \t\tstrbuf_release(&buf);\n-\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n+\t\tformat_commit_message(commit, format, &buf, &ctx);\n \t\treturn strbuf_detach(&buf, NULL);\n \t}\n \tdie(_(\"No existing author found with '%s'\"), name);\n@@ -1428,6 +1441,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Commit message options\")),\n \t\tOPT_FILENAME('F', \"file\", &logfile, N_(\"read message from file\")),\n \t\tOPT_STRING(0, \"author\", &force_author, N_(\"author\"), N_(\"override author for commit\")),\n+\t\tOPT_BOOLEAN(0, \"use-mailmap\", &mailmap, N_(\"Use mailmap file when searching for author\")),\n \t\tOPT_STRING(0, \"date\", &force_date, N_(\"date\"), N_(\"override date for commit\")),\n \t\tOPT_CALLBACK('m', \"message\", &message, N_(\"message\"), N_(\"commit message\"), opt_parse_m),\n \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n--\n1.8.4.rc4.1.g0d8beaa.dirty\n"},{"id":"225726","messageId":"xmqqbo4opajg.fsf@gitster.dls.corp.google.com","threadId":"34750","inReplyTo":"1377265711-11492-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] git-commit: search author pattern against mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-23T17:44:03Z","receivedAt":"2013-08-23T17:44:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> When committing for someone else, using the --author option, it can be\n> nice to use the mailmap file to find the correct name spelling and email\n> address.\n>\n> Currently, you would have to find the correct mapping in mailmap file\n> first, and then use the full ident form when committing.\n>\n> Let's allow git-commit to find if an entry exists in mailmap file for\n> that pattern, and use that instead.\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n> Hi,\n> I would use that feature at work where I happen to commit some work for\n> other colleagues, while we heavily rely on mailmap file to have decent indents.\n>\n> On the other hand, I'm kind of embarrassed to add this new option to\n> git-commit.\n\nMy initial reaction was \"Why should something as important as 'git\ncommit' should be playing a guessing-game?\" ;-) and I am kind of\nashamed to have added 146ea068 (git commit --author=$name: look\n$name up in existing commits, 2008-08-26) and then am embarrased to\nhave completely forgotten about it. I never use the feature myself.\n\nBut for that old and established \"--author parameter that does not\nuse the standard format guesses\" feature to be useful, I agree that\nit should honor the mailmap.\n\nI wonder if it would hurt anybody if we made this unconditional, not\neven with \"--no-mailmap\" override? Opinions?\n\n>\n>  Documentation/git-commit.txt |  6 +++++-\n>  builtin/commit.c             | 16 +++++++++++++++-\n>  2 files changed, 20 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index 1a7616c..9e3fe04 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -12,7 +12,7 @@ SYNOPSIS\n>  \t   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]\n>  \t   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\n>  \t   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\n> -\t   [--date=<date>] [--cleanup=<mode>] [--[no-]status]\n> +\t   [--use-mailmap] [--date=<date>] [--cleanup=<mode>] [--[no-]status]\n>  \t   [-i | -o] [-S[<keyid>]] [--] [<file>...]\n>\n>  DESCRIPTION\n> @@ -131,6 +131,10 @@ OPTIONS\n>  \tcommit by that author (i.e. rev-list --all -i --author=<author>);\n>  \tthe commit author is then copied from the first such commit found.\n>\n> +--use-mailmap::\n> +\tWhen used with `--author=<author>`, match the <author> pattern\n> +\tagainst mapped name and email. See linkgit:git-shortlog[1].\n> +\n>  --date=<date>::\n>  \tOverride the author date used in the commit.\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 10acc53..fbd0664 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -30,6 +30,7 @@\n>  #include \"column.h\"\n>  #include \"sequencer.h\"\n>  #include \"notes-utils.h\"\n> +#include \"mailmap.h\"\n>\n>  static const char * const builtin_commit_usage[] = {\n>  \tN_(\"git commit [options] [--] <pathspec>...\"),\n> @@ -87,6 +88,7 @@ static enum {\n>  } commit_style;\n>\n>  static const char *logfile, *force_author;\n> +static int mailmap;\n>  static const char *template_file;\n>  /*\n>   * The _message variables are commit names from which to take\n> @@ -945,13 +947,24 @@ static const char *find_author_by_nickname(const char *name)\n>  \tav[++ac] = buf.buf;\n>  \tav[++ac] = NULL;\n>  \tsetup_revisions(ac, av, &revs, NULL);\n> +\tif (mailmap) {\n> +\t\trevs.mailmap = xcalloc(1, sizeof(struct string_list));\n> +\t\tread_mailmap(revs.mailmap, NULL);\n> +\t}\n>  \tprepare_revision_walk(&revs);\n>  \tcommit = get_revision(&revs);\n>  \tif (commit) {\n>  \t\tstruct pretty_print_context ctx = {0};\n> +\t\tconst char *format;\n> +\n> +\t\tif (mailmap)\n> +\t\t\tformat = \"%aN <%aE>\";\n> +\t\telse\n> +\t\t\tformat = \"%an <%ae>\";\n> +\n>  \t\tctx.date_mode = DATE_NORMAL;\n>  \t\tstrbuf_release(&buf);\n> -\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n> +\t\tformat_commit_message(commit, format, &buf, &ctx);\n>  \t\treturn strbuf_detach(&buf, NULL);\n>  \t}\n>  \tdie(_(\"No existing author found with '%s'\"), name);\n> @@ -1428,6 +1441,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_GROUP(N_(\"Commit message options\")),\n>  \t\tOPT_FILENAME('F', \"file\", &logfile, N_(\"read message from file\")),\n>  \t\tOPT_STRING(0, \"author\", &force_author, N_(\"author\"), N_(\"override author for commit\")),\n> +\t\tOPT_BOOLEAN(0, \"use-mailmap\", &mailmap, N_(\"Use mailmap file when searching for author\")),\n>  \t\tOPT_STRING(0, \"date\", &force_date, N_(\"date\"), N_(\"override date for commit\")),\n>  \t\tOPT_CALLBACK('m', \"message\", &message, N_(\"message\"), N_(\"commit message\"), opt_parse_m),\n>  \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n> --\n> 1.8.4.rc4.1.g0d8beaa.dirty\n"},{"id":"225731","messageId":"20130823183541.GB30130@sigill.intra.peff.net","threadId":"34750","inReplyTo":"xmqqbo4opajg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-commit: search author pattern against mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-23T18:35:41Z","receivedAt":"2013-08-23T18:35:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 23, 2013 at 10:44:03AM -0700, Junio C Hamano wrote:\n\n> My initial reaction was \"Why should something as important as 'git\n> commit' should be playing a guessing-game?\" ;-) and I am kind of\n> ashamed to have added 146ea068 (git commit --author=$name: look\n> $name up in existing commits, 2008-08-26) and then am embarrased to\n> have completely forgotten about it. I never use the feature myself.\n> \n> But for that old and established \"--author parameter that does not\n> use the standard format guesses\" feature to be useful, I agree that\n> it should honor the mailmap.\n> \n> I wonder if it would hurt anybody if we made this unconditional, not\n> even with \"--no-mailmap\" override? Opinions?\n\nI think it would be OK. You can always override by giving the actual\nfull address you want instead of a partial one. And if somebody is not\nup to date in the .mailmap file, maybe this would be a good hint that\nyou should take care of that. :)\n\nI paused for a second, thinking that such advice might not be good for\npeople who do not want to make an official change to upstream's\n.mailmap (e.g., because they do not want to pollute a long-running fork\nthat will need to merge from upstream, or do not want to pollute a topic\nbranch with an unrelated commit). But I forgot that we have\nmailmap.file, if they want something custom.\n\nSo I think anyone for whom the mailmap lookup does not provide the right\nanswer will fall into one of two groups:\n\n  1. A one-off, which can be overridden by specifying the address you\n     do want.\n\n  2. Somebody you will be mentioning frequently; bother to set up\n     a mailmap.file.\n\nAs an aside, it seems silly that we do not respect $GIT_DIR/mailmap by\ndefault, even without a config option. But I doubt that anybody cares\ntoo much, if nobody has raised the issue in all of these years.\n\n-Peff\n"},{"id":"225735","messageId":"xmqqwqncnsaz.fsf@gitster.dls.corp.google.com","threadId":"34750","inReplyTo":"20130823183541.GB30130@sigill.intra.peff.net","subject":"Re: [PATCH] git-commit: search author pattern against mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-23T19:03:16Z","receivedAt":"2013-08-23T19:03:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> But for that old and established \"--author parameter that does not\n>> use the standard format guesses\" feature to be useful, I agree that\n>> it should honor the mailmap.\n>> \n>> I wonder if it would hurt anybody if we made this unconditional, not\n>> even with \"--no-mailmap\" override? Opinions?\n>\n> I think it would be OK. You can always override by giving the actual\n> full address you want instead of a partial one.\n\nOK, so how about labelling it as a bugfix, like this perhaps?  We\nobviously need a test or two, though.\n\n-- >8 --\nFrom: Antoine Pelisse <apelisse@gmail.com>\nDate: Fri, 23 Aug 2013 15:48:31 +0200\nSubject: [PATCH] commit: search author pattern against mailmap\n\n\"git commit --author=$name\" sets the author to one whose name\nmatches the given string from existing commits, when $name is not in\nthe \"Name <e-mail>\" format. However, it does not honor the mailmap\nto use the canonical name for the author found this way.\n\nFix it by telling the logic to find a matching existing author to\nhonor the mailmap, and use the name and email after applying the\nmailmap.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/commit.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 10acc53..5b7d969 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -30,6 +30,7 @@\n #include \"column.h\"\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n+#include \"mailmap.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <pathspec>...\"),\n@@ -945,13 +946,16 @@ static const char *find_author_by_nickname(const char *name)\n \tav[++ac] = buf.buf;\n \tav[++ac] = NULL;\n \tsetup_revisions(ac, av, &revs, NULL);\n+\trevs.mailmap = xcalloc(1, sizeof(struct string_list));\n+\tread_mailmap(revs.mailmap, NULL);\n+\n \tprepare_revision_walk(&revs);\n \tcommit = get_revision(&revs);\n \tif (commit) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.date_mode = DATE_NORMAL;\n \t\tstrbuf_release(&buf);\n-\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n+\t\tformat_commit_message(commit, \"%aN <%aE>\", &buf, &ctx);\n \t\treturn strbuf_detach(&buf, NULL);\n \t}\n \tdie(_(\"No existing author found with '%s'\"), name);\n-- \n1.8.4-rc4-299-g7e07a8d\n"},{"id":"225740","messageId":"CALWbr2x28wrzxJ=M6meCX8G0Bh4ObvHkYGqfGTNwPjWMxgJjQg@mail.gmail.com","threadId":"34750","inReplyTo":"xmqqwqncnsaz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-23T19:47:37Z","receivedAt":"2013-08-23T19:47:37Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Fri, Aug 23, 2013 at 9:03 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> OK, so how about labelling it as a bugfix, like this perhaps?  We\n> obviously need a test or two, though.\n\nOK,\nI will resubmit tomorrow with some tests.\n"},{"id":"225746","messageId":"xmqqsiy0nnlr.fsf@gitster.dls.corp.google.com","threadId":"34750","inReplyTo":"CALWbr2x28wrzxJ=M6meCX8G0Bh4ObvHkYGqfGTNwPjWMxgJjQg@mail.gmail.com","subject":"Re: [PATCH] git-commit: search author pattern against mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-23T20:44:48Z","receivedAt":"2013-08-23T20:44:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> On Fri, Aug 23, 2013 at 9:03 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> OK, so how about labelling it as a bugfix, like this perhaps?  We\n>> obviously need a test or two, though.\n>\n> OK,\n> I will resubmit tomorrow with some tests.\n\nThanks.\n\nAlso, after I sent out that \"like this\" patch, I realized that the\nmailmap string-list no longer has to be on the heap if we are doing\nit unconditionally (it can be an auto variable in the function,\nsitting next to \"struct strbuf buf\").\n"},{"id":"225792","messageId":"1377353267-3886-1-git-send-email-apelisse@gmail.com","threadId":"34750","inReplyTo":"xmqqsiy0nnlr.fsf@gitster.dls.corp.google.com","subject":"[PATCH] commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-24T14:07:47Z","receivedAt":"2013-08-24T14:07:47Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"\"git commit --author=$name\" sets the author to one whose name\nmatches the given string from existing commits, when $name is not in\nthe \"Name <e-mail>\" format. However, it does not honor the mailmap\nto use the canonical name for the author found this way.\n\nFix it by telling the logic to find a matching existing author to\nhonor the mailmap, and use the name and email after applying the\nmailmap.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n builtin/commit.c   |  7 ++++++-\n t/t4203-mailmap.sh | 11 +++++++++++\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 10acc53..21e0f95 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -30,6 +30,7 @@\n #include \"column.h\"\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n+#include \"mailmap.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <pathspec>...\"),\n@@ -935,6 +936,7 @@ static const char *find_author_by_nickname(const char *name)\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n \tconst char *av[20];\n \tint ac = 0;\n \n@@ -945,13 +947,16 @@ static const char *find_author_by_nickname(const char *name)\n \tav[++ac] = buf.buf;\n \tav[++ac] = NULL;\n \tsetup_revisions(ac, av, &revs, NULL);\n+\trevs.mailmap = &mailmap;\n+\tread_mailmap(revs.mailmap, NULL);\n+\n \tprepare_revision_walk(&revs);\n \tcommit = get_revision(&revs);\n \tif (commit) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.date_mode = DATE_NORMAL;\n \t\tstrbuf_release(&buf);\n-\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n+\t\tformat_commit_message(commit, \"%aN <%aE>\", &buf, &ctx);\n \t\treturn strbuf_detach(&buf, NULL);\n \t}\n \tdie(_(\"No existing author found with '%s'\"), name);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex baa4685..4d715f0 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -470,4 +470,15 @@ test_expect_success 'Blame output (complex mapping)' '\n \ttest_cmp expect actual.fuzz\n '\n \n+cat >expect <<\\EOF\n+Some Dude <some@dude.xx>\n+EOF\n+\n+test_expect_success 'commit --author honors mailmap' '\n+\ttest_must_fail git commit --author \"nick\" --allow-empty -meight &&\n+\tgit commit --author \"Some Dude\" --allow-empty -meight &&\n+\tgit show --pretty=format:\"%an <%ae>%n\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.4.rc4.2.g8483dfa\n"},{"id":"225817","messageId":"20130825040122.GA18676@sigill.intra.peff.net","threadId":"34750","inReplyTo":"1377353267-3886-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-25T04:01:22Z","receivedAt":"2013-08-25T04:01:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 24, 2013 at 04:07:47PM +0200, Antoine Pelisse wrote:\n\n> @@ -945,13 +947,16 @@ static const char *find_author_by_nickname(const char *name)\n>  \tav[++ac] = buf.buf;\n>  \tav[++ac] = NULL;\n>  \tsetup_revisions(ac, av, &revs, NULL);\n> +\trevs.mailmap = &mailmap;\n> +\tread_mailmap(revs.mailmap, NULL);\n> +\n>  \tprepare_revision_walk(&revs);\n>  \tcommit = get_revision(&revs);\n>  \tif (commit) {\n>  \t\tstruct pretty_print_context ctx = {0};\n>  \t\tctx.date_mode = DATE_NORMAL;\n>  \t\tstrbuf_release(&buf);\n> -\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n> +\t\tformat_commit_message(commit, \"%aN <%aE>\", &buf, &ctx);\n>  \t\treturn strbuf_detach(&buf, NULL);\n>  \t}\n>  \tdie(_(\"No existing author found with '%s'\"), name);\n\nDo we need to clear_mailmap before returning to avoid a leak?\n\nI suspect we may be leaking pending commits from the revision walker,\ntoo, but I'm not sure we have an easy \"clear everything\" function there.\n\n-Peff\n"},{"id":"225826","messageId":"xmqqob8ml588.fsf@gitster.dls.corp.google.com","threadId":"34750","inReplyTo":"20130825040122.GA18676@sigill.intra.peff.net","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-25T05:16:55Z","receivedAt":"2013-08-25T05:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Aug 24, 2013 at 04:07:47PM +0200, Antoine Pelisse wrote:\n>\n>> @@ -945,13 +947,16 @@ static const char *find_author_by_nickname(const char *name)\n>>  \tav[++ac] = buf.buf;\n>>  \tav[++ac] = NULL;\n>>  \tsetup_revisions(ac, av, &revs, NULL);\n>> +\trevs.mailmap = &mailmap;\n>> +\tread_mailmap(revs.mailmap, NULL);\n>> +\n>>  \tprepare_revision_walk(&revs);\n>>  \tcommit = get_revision(&revs);\n>>  \tif (commit) {\n>>  \t\tstruct pretty_print_context ctx = {0};\n>>  \t\tctx.date_mode = DATE_NORMAL;\n>>  \t\tstrbuf_release(&buf);\n>> -\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n>> +\t\tformat_commit_message(commit, \"%aN <%aE>\", &buf, &ctx);\n>>  \t\treturn strbuf_detach(&buf, NULL);\n>>  \t}\n>>  \tdie(_(\"No existing author found with '%s'\"), name);\n>\n> Do we need to clear_mailmap before returning to avoid a leak?\n\nGood question. What I queued yesterday seems to have a call to\nclear_mailmap(&mailmap) before that return.\n"},{"id":"225853","messageId":"CALWbr2w0C77j-Qw0L03dT04pm81iz0sn-W8+=t7271nhCW=OYw@mail.gmail.com","threadId":"34750","inReplyTo":"xmqqob8ml588.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-25T09:47:34Z","receivedAt":"2013-08-25T09:47:34Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Sun, Aug 25, 2013 at 7:16 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jeff King <peff@peff.net> writes:\n>> Do we need to clear_mailmap before returning to avoid a leak?\n>\n> Good question. What I queued yesterday seems to have a call to\n> clear_mailmap(&mailmap) before that return.\n\nIndeed, the version you queued has clear_mailmap(), not the version\nyou sent by email.\n\nWill resend.\n\nThanks,\n"},{"id":"225855","messageId":"1377424889-15399-1-git-send-email-apelisse@gmail.com","threadId":"34750","inReplyTo":"xmqqob8ml588.fsf@gitster.dls.corp.google.com","subject":"[PATCH] commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-25T10:01:29Z","receivedAt":"2013-08-25T10:01:29Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"\"git commit --author=$name\" sets the author to one whose name\nmatches the given string from existing commits, when $name is not in\nthe \"Name <e-mail>\" format. However, it does not honor the mailmap\nto use the canonical name for the author found this way.\n\nFix it by telling the logic to find a matching existing author to\nhonor the mailmap, and use the name and email after applying the\nmailmap.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nHey,\nSo I kept clear_mailmap() where you put it, but I think it could be moved\nright after get_revision(). That is because I think format_commit_message()\nwill run another read_mailmap() with an heap-allocated string_list.\nAnyway, I'm not sure it makes a big difference here.\n\nThanks,\nAntoine\n\n builtin/commit.c   |  8 +++++++-\n t/t4203-mailmap.sh | 11 +++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 10acc53..a48a7fe 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -30,6 +30,7 @@\n #include \"column.h\"\n #include \"sequencer.h\"\n #include \"notes-utils.h\"\n+#include \"mailmap.h\"\n\n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <pathspec>...\"),\n@@ -935,6 +936,7 @@ static const char *find_author_by_nickname(const char *name)\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n \tconst char *av[20];\n \tint ac = 0;\n\n@@ -945,13 +947,17 @@ static const char *find_author_by_nickname(const char *name)\n \tav[++ac] = buf.buf;\n \tav[++ac] = NULL;\n \tsetup_revisions(ac, av, &revs, NULL);\n+\trevs.mailmap = &mailmap;\n+\tread_mailmap(revs.mailmap, NULL);\n+\n \tprepare_revision_walk(&revs);\n \tcommit = get_revision(&revs);\n \tif (commit) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tctx.date_mode = DATE_NORMAL;\n \t\tstrbuf_release(&buf);\n-\t\tformat_commit_message(commit, \"%an <%ae>\", &buf, &ctx);\n+\t\tformat_commit_message(commit, \"%aN <%aE>\", &buf, &ctx);\n+\t\tclear_mailmap(&mailmap);\n \t\treturn strbuf_detach(&buf, NULL);\n \t}\n \tdie(_(\"No existing author found with '%s'\"), name);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex baa4685..4d715f0 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -470,4 +470,15 @@ test_expect_success 'Blame output (complex mapping)' '\n \ttest_cmp expect actual.fuzz\n '\n\n+cat >expect <<\\EOF\n+Some Dude <some@dude.xx>\n+EOF\n+\n+test_expect_success 'commit --author honors mailmap' '\n+\ttest_must_fail git commit --author \"nick\" --allow-empty -meight &&\n+\tgit commit --author \"Some Dude\" --allow-empty -meight &&\n+\tgit show --pretty=format:\"%an <%ae>%n\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n--\n1.8.4.rc4.2.g8483dfa.dirty\n"},{"id":"225856","messageId":"20130825103041.GB12556@sigill.intra.peff.net","threadId":"34750","inReplyTo":"1377424889-15399-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-25T10:30:41Z","receivedAt":"2013-08-25T10:30:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 25, 2013 at 12:01:29PM +0200, Antoine Pelisse wrote:\n\n> So I kept clear_mailmap() where you put it, but I think it could be moved\n> right after get_revision(). That is because I think format_commit_message()\n> will run another read_mailmap() with an heap-allocated string_list.\n> Anyway, I'm not sure it makes a big difference here.\n\nYeah, format_commit_message does not even pay attention to our\nrevs.mailmap.\n\nIt does make me wonder if there should simply be a static singleton\nmailmap that gets loaded once per program invocation and then cleaned up\nat exit. That is clearly what format_commit_message is doing. Is there\nactually a use case for having a custom one in rev_info? It's not like\nyou can even control where it reads from when you call read_mailmap.\n\nI guess we need it as a boolean \"do we want to mailmap at all\" for the\nregular pretty formats, but it could just be a flag in rev_info instead\nof a pointer.\n\n-Peff\n"},{"id":"225866","messageId":"CALWbr2zfpZYGri9aGL3DGhadnYF=0xx_h95ZjN7S4beoAES68A@mail.gmail.com","threadId":"34750","inReplyTo":"20130825103041.GB12556@sigill.intra.peff.net","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-25T13:37:24Z","receivedAt":"2013-08-25T13:37:24Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Sun, Aug 25, 2013 at 12:30 PM, Jeff King <peff@peff.net> wrote:\n> On Sun, Aug 25, 2013 at 12:01:29PM +0200, Antoine Pelisse wrote:\n>\n>> So I kept clear_mailmap() where you put it, but I think it could be moved\n>> right after get_revision(). That is because I think format_commit_message()\n>> will run another read_mailmap() with an heap-allocated string_list.\n>> Anyway, I'm not sure it makes a big difference here.\n>\n> Yeah, format_commit_message does not even pay attention to our\n> revs.mailmap.\n>\n> It does make me wonder if there should simply be a static singleton\n> mailmap that gets loaded once per program invocation and then cleaned up\n> at exit. That is clearly what format_commit_message is doing. Is there\n> actually a use case for having a custom one in rev_info? It's not like\n> you can even control where it reads from when you call read_mailmap.\n>\n> I guess we need it as a boolean \"do we want to mailmap at all\" for the\n> regular pretty formats, but it could just be a flag in rev_info instead\n> of a pointer.\n\nSo we would stop passing mailmap string_list along down to map_user(),\nand the mailmap file (or blob) would be read the first time it's\nneeded, and stored in a static global variable in mailmap.c. I think\nI'm OK with that because I don't think it would make sense to have\nmultiple instances of a mailmap string_list in the same git-command\ninstance.\n\nWho would be responsible for deleting the string_list ? It would\neither be done in each command, or done through a atexit(3) registered\nfunction (but then, why would we even care about cleaning it up?).\n"},{"id":"225868","messageId":"20130825165153.GC21092@sigill.intra.peff.net","threadId":"34750","inReplyTo":"CALWbr2zfpZYGri9aGL3DGhadnYF=0xx_h95ZjN7S4beoAES68A@mail.gmail.com","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-25T16:51:53Z","receivedAt":"2013-08-25T16:51:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 25, 2013 at 03:37:24PM +0200, Antoine Pelisse wrote:\n\n> So we would stop passing mailmap string_list along down to map_user(),\n> and the mailmap file (or blob) would be read the first time it's\n> needed, and stored in a static global variable in mailmap.c. I think\n> I'm OK with that because I don't think it would make sense to have\n> multiple instances of a mailmap string_list in the same git-command\n> instance.\n\nExactly. Sample (largely untested) patch is below if you want to use it\nas a starting point. There are probably a few additional cleanups on top\n(e.g., \"git log\" understands \"--mailmap\", which should probably be\ncentralized to handle_revision_opt).\n\nI'm on the fence. It doesn't actually save that many lines of code, and\nI guess it's possible that somebody would want a custom mailmap in the\nfuture. Even though you can't do it right now, all it would take is\nexposing read_mailmap_file and read_mailmap_blob outside of mailmap.c.\nOf course, it would be easy to expose map_user_from at the same time.\n\n> Who would be responsible for deleting the string_list ? It would\n> either be done in each command, or done through a atexit(3) registered\n> function (but then, why would we even care about cleaning it up?).\n\nExactly. You would clean it up at exit, but the OS does it for you\nalready.\n\n-Peff\n\n---\n builtin/blame.c         |  7 +------\n builtin/check-mailmap.c | 12 ++++--------\n builtin/log.c           |  5 +----\n builtin/shortlog.c      |  7 ++-----\n commit.h                |  2 +-\n log-tree.c              |  2 +-\n mailmap.c               | 31 ++++++++++++++++++++++++++++---\n mailmap.h               |  7 +++----\n pretty.c                | 17 +++--------------\n revision.c              | 10 +++++-----\n revision.h              |  2 +-\n shortlog.h              |  2 --\n 12 files changed, 50 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 079dcd3..680adaf 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -48,8 +48,6 @@ static size_t blame_date_width;\n static enum date_mode blame_date_mode = DATE_ISO8601;\n static size_t blame_date_width;\n \n-static struct string_list mailmap;\n-\n #ifndef DEBUG\n #define DEBUG 0\n #endif\n@@ -1394,8 +1392,7 @@ static void get_ac_line(const char *inbuf, const char *what,\n \t/*\n \t * Now, convert both name and e-mail using mailmap\n \t */\n-\tmap_user(&mailmap, &mailbuf, &maillen,\n-\t\t &namebuf, &namelen);\n+\tmap_user(&mailbuf, &maillen, &namebuf, &namelen);\n \n \tstrbuf_addf(mail, \"<%.*s>\", (int)maillen, mailbuf);\n \tstrbuf_add(name, namebuf, namelen);\n@@ -2512,8 +2509,6 @@ parse_done:\n \tsb.ent = ent;\n \tsb.path = path;\n \n-\tread_mailmap(&mailmap, NULL);\n-\n \tif (!incremental)\n \t\tsetup_pager();\n \ndiff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\nindex 8f4d809..b3a13f4 100644\n--- a/builtin/check-mailmap.c\n+++ b/builtin/check-mailmap.c\n@@ -14,7 +14,7 @@ static const struct option check_mailmap_options[] = {\n \tOPT_END()\n };\n \n-static void check_mailmap(struct string_list *mailmap, const char *contact)\n+static void check_mailmap(const char *contact)\n {\n \tconst char *name, *mail;\n \tsize_t namelen, maillen;\n@@ -28,7 +28,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)\n \tmail = ident.mail_begin;\n \tmaillen = ident.mail_end - ident.mail_begin;\n \n-\tmap_user(mailmap, &mail, &maillen, &name, &namelen);\n+\tmap_user(&mail, &maillen, &name, &namelen);\n \n \tif (namelen)\n \t\tprintf(\"%.*s \", (int)namelen, name);\n@@ -38,7 +38,6 @@ int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n \n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, check_mailmap_options,\n@@ -46,21 +45,18 @@ int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n \tif (argc == 0 && !use_stdin)\n \t\tdie(_(\"no contacts specified\"));\n \n-\tread_mailmap(&mailmap, NULL);\n-\n \tfor (i = 0; i < argc; ++i)\n-\t\tcheck_mailmap(&mailmap, argv[i]);\n+\t\tcheck_mailmap(argv[i]);\n \tmaybe_flush_or_die(stdout, \"stdout\");\n \n \tif (use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\twhile (strbuf_getline(&buf, stdin, '\\n') != EOF) {\n-\t\t\tcheck_mailmap(&mailmap, buf.buf);\n+\t\t\tcheck_mailmap(buf.buf);\n \t\t\tmaybe_flush_or_die(stdout, \"stdout\");\n \t\t}\n \t\tstrbuf_release(&buf);\n \t}\n \n-\tclear_mailmap(&mailmap);\n \treturn 0;\n }\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 2625f98..88ebd85 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -168,10 +168,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tif (source)\n \t\trev->show_source = 1;\n \n-\tif (mailmap) {\n-\t\trev->mailmap = xcalloc(1, sizeof(struct string_list));\n-\t\tread_mailmap(rev->mailmap, NULL);\n-\t}\n+\trev->use_mailmap = mailmap;\n \n \tif (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {\n \t\t/*\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 1434f8f..22010b3 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -31,7 +31,7 @@ static void insert_one_record(struct shortlog *log,\n \t\t\t      const char *author,\n \t\t\t      const char *oneline)\n {\n-\tconst char *dot3 = log->common_repo_prefix;\n+\tconst char *dot3 = mailmap_repo_abbrev();\n \tchar *buffer, *p;\n \tstruct string_list_item *item;\n \tconst char *mailbuf, *namebuf;\n@@ -49,7 +49,7 @@ static void insert_one_record(struct shortlog *log,\n \tnamelen = ident.name_end - ident.name_begin;\n \tmaillen = ident.mail_end - ident.mail_begin;\n \n-\tmap_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n+\tmap_user(&mailbuf, &maillen, &namebuf, &namelen);\n \tstrbuf_add(&namemailbuf, namebuf, namelen);\n \n \tif (log->email)\n@@ -209,8 +209,6 @@ void shortlog_init(struct shortlog *log)\n {\n \tmemset(log, 0, sizeof(*log));\n \n-\tread_mailmap(&log->mailmap, &log->common_repo_prefix);\n-\n \tlog->list.strdup_strings = 1;\n \tlog->wrap = DEFAULT_WRAPLEN;\n \tlog->in1 = DEFAULT_INDENT1;\n@@ -323,5 +321,4 @@ void shortlog_output(struct shortlog *log)\n \tstrbuf_release(&sb);\n \tlog->list.strdup_strings = 1;\n \tstring_list_clear(&log->list, 1);\n-\tclear_mailmap(&log->mailmap);\n }\ndiff --git a/commit.h b/commit.h\nindex d912a9d..d391a53 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -94,7 +94,7 @@ struct pretty_print_context {\n \tchar *notes_message;\n \tstruct reflog_walk_info *reflog_info;\n \tconst char *output_encoding;\n-\tstruct string_list *mailmap;\n+\tint use_mailmap;\n \tint color;\n \tstruct ident_split *from_ident;\n \ndiff --git a/log-tree.c b/log-tree.c\nindex a49d8e8..ed77e61 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -615,7 +615,7 @@ void show_log(struct rev_info *opt)\n \tctx.preserve_subject = opt->preserve_subject;\n \tctx.reflog_info = opt->reflog_info;\n \tctx.fmt = opt->commit_format;\n-\tctx.mailmap = opt->mailmap;\n+\tctx.use_mailmap = opt->use_mailmap;\n \tctx.color = opt->diffopt.use_color;\n \tctx.output_encoding = get_log_output_encoding();\n \tif (opt->from_ident.mail_begin && opt->from_ident.name_begin)\ndiff --git a/mailmap.c b/mailmap.c\nindex 44614fc..05540fa 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -13,6 +13,9 @@ const char *git_mailmap_blob;\n \n const char *git_mailmap_file;\n const char *git_mailmap_blob;\n+static struct string_list the_mailmap;\n+static char *repo_abbrev;\n+static int initialized;\n \n struct mailmap_info {\n \tchar *name;\n@@ -28,6 +31,15 @@ struct mailmap_entry {\n \tstruct string_list namemap;\n };\n \n+static void init_mailmap(void)\n+{\n+\tif (initialized)\n+\t\treturn;\n+\n+\tread_mailmap(&the_mailmap, &repo_abbrev);\n+\tinitialized = 1;\n+}\n+\n static void free_mailmap_info(void *p, const char *s)\n {\n \tstruct mailmap_info *mi = (struct mailmap_info *)p;\n@@ -311,9 +323,9 @@ static struct string_list_item *lookup_prefix(struct string_list *map,\n \treturn NULL;\n }\n \n-int map_user(struct string_list *map,\n-\t     const char **email, size_t *emaillen,\n-\t     const char **name, size_t *namelen)\n+static int map_user_from(struct string_list *map,\n+\t\t\t const char **email, size_t *emaillen,\n+\t\t\t const char **name, size_t *namelen)\n {\n \tstruct string_list_item *item;\n \tstruct mailmap_entry *me;\n@@ -359,3 +371,16 @@ int map_user(struct string_list *map,\n \tdebug_mm(\"map_user:  --\\n\");\n \treturn 0;\n }\n+\n+int map_user(const char **email, size_t *emaillen,\n+\t     const char **name, size_t *namelen)\n+{\n+\tinit_mailmap();\n+\treturn map_user_from(&the_mailmap, email, emaillen, name, namelen);\n+}\n+\n+const char *mailmap_repo_abbrev(void)\n+{\n+\tinit_mailmap();\n+\treturn repo_abbrev;\n+}\ndiff --git a/mailmap.h b/mailmap.h\nindex ed7c93b..de52e63 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -1,10 +1,9 @@ void clear_mailmap(struct string_list *map);\n #ifndef MAILMAP_H\n #define MAILMAP_H\n \n-int read_mailmap(struct string_list *map, char **repo_abbrev);\n-void clear_mailmap(struct string_list *map);\n+int map_user(const char **email, size_t *emaillen,\n+\t     const char **name, size_t *namelen);\n \n-int map_user(struct string_list *map,\n-\t\t\t const char **email, size_t *emaillen, const char **name, size_t *namelen);\n+const char *mailmap_repo_abbrev(void);\n \n #endif\ndiff --git a/pretty.c b/pretty.c\nindex 74563c9..94a9628 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -428,8 +428,8 @@ void pp_user_info(struct pretty_print_context *pp,\n \tnamebuf = ident.name_begin;\n \tnamelen = ident.name_end - ident.name_begin;\n \n-\tif (pp->mailmap)\n-\t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n+\tif (pp->use_mailmap)\n+\t\tmap_user(&mailbuf, &maillen, &namebuf, &namelen);\n \n \tif (pp->fmt == CMIT_FMT_EMAIL) {\n \t\tif (pp->from_ident) {\n@@ -688,17 +688,6 @@ void logmsg_free(char *msg, const struct commit *commit)\n \t\tfree(msg);\n }\n \n-static int mailmap_name(const char **email, size_t *email_len,\n-\t\t\tconst char **name, size_t *name_len)\n-{\n-\tstatic struct string_list *mail_map;\n-\tif (!mail_map) {\n-\t\tmail_map = xcalloc(1, sizeof(*mail_map));\n-\t\tread_mailmap(mail_map, NULL);\n-\t}\n-\treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n-}\n-\n static size_t format_person_part(struct strbuf *sb, char part,\n \t\t\t\t const char *msg, int len, enum date_mode dmode)\n {\n@@ -717,7 +706,7 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \tmaillen = s.mail_end - s.mail_begin;\n \n \tif (part == 'N' || part == 'E') /* mailmap lookup */\n-\t\tmailmap_name(&mail, &maillen, &name, &namelen);\n+\t\tmap_user(&mail, &maillen, &name, &namelen);\n \tif (part == 'n' || part == 'N') {\t/* name */\n \t\tstrbuf_add(sb, name, namelen);\n \t\treturn placeholder_len;\ndiff --git a/revision.c b/revision.c\nindex 84ccc05..5f96316 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2661,7 +2661,7 @@ int rewrite_parents(struct rev_info *revs, struct commit *commit,\n \treturn 0;\n }\n \n-static int commit_rewrite_person(struct strbuf *buf, const char *what, struct string_list *mailmap)\n+static int commit_rewrite_person(struct strbuf *buf, const char *what)\n {\n \tchar *person, *endp;\n \tsize_t len, namelen, maillen;\n@@ -2688,7 +2688,7 @@ static int commit_rewrite_person(struct strbuf *buf, const char *what, struct st\n \tname = ident.name_begin;\n \tnamelen = ident.name_end - ident.name_begin;\n \n-\tif (map_user(mailmap, &mail, &maillen, &name, &namelen)) {\n+\tif (map_user(&mail, &maillen, &name, &namelen)) {\n \t\tstruct strbuf namemail = STRBUF_INIT;\n \n \t\tstrbuf_addf(&namemail, \"%.*s <%.*s>\",\n@@ -2737,12 +2737,12 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n \tif (buf.len)\n \t\tstrbuf_addstr(&buf, message);\n \n-\tif (opt->grep_filter.header_list && opt->mailmap) {\n+\tif (opt->grep_filter.header_list && opt->use_mailmap) {\n \t\tif (!buf.len)\n \t\t\tstrbuf_addstr(&buf, message);\n \n-\t\tcommit_rewrite_person(&buf, \"\\nauthor \", opt->mailmap);\n-\t\tcommit_rewrite_person(&buf, \"\\ncommitter \", opt->mailmap);\n+\t\tcommit_rewrite_person(&buf, \"\\nauthor \");\n+\t\tcommit_rewrite_person(&buf, \"\\ncommitter \");\n \t}\n \n \t/* Append \"fake\" message parts as needed */\ndiff --git a/revision.h b/revision.h\nindex 95859ba..a79817e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -152,7 +152,7 @@ struct rev_info {\n \tconst char\t*subject_prefix;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n-\tstruct string_list *mailmap;\n+\tint use_mailmap;\n \n \t/* Filter by commit log message */\n \tstruct grep_opt\tgrep_filter;\ndiff --git a/shortlog.h b/shortlog.h\nindex de4f86f..e6c3055 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -14,9 +14,7 @@ struct shortlog {\n \tint user_format;\n \tint abbrev;\n \n-\tchar *common_repo_prefix;\n \tint email;\n-\tstruct string_list mailmap;\n };\n \n void shortlog_init(struct shortlog *log);\n"},{"id":"225875","messageId":"CALWbr2xRZzwUKUFZ=v21h6h1c13Hk8V2VgMQiQwjxvdKQ=CcDA@mail.gmail.com","threadId":"34750","inReplyTo":"20130825165153.GC21092@sigill.intra.peff.net","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-25T20:42:34Z","receivedAt":"2013-08-25T20:42:34Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Sun, Aug 25, 2013 at 6:51 PM, Jeff King <peff@peff.net> wrote:\n> On Sun, Aug 25, 2013 at 03:37:24PM +0200, Antoine Pelisse wrote:\n>\n>> So we would stop passing mailmap string_list along down to map_user(),\n>> and the mailmap file (or blob) would be read the first time it's\n>> needed, and stored in a static global variable in mailmap.c. I think\n>> I'm OK with that because I don't think it would make sense to have\n>> multiple instances of a mailmap string_list in the same git-command\n>> instance.\n>\n> Exactly. Sample (largely untested) patch is below if you want to use it\n> as a starting point. There are probably a few additional cleanups on top\n> (e.g., \"git log\" understands \"--mailmap\", which should probably be\n> centralized to handle_revision_opt).\n\nI'm not exactly sure how I would improve the patch you sent. I\nremember Junio was not willing to move --use-mailmap option to\nrevision options and wanted to keep it just for \"log\" (though I don't\nhave a reference to that email).\n\nI've tested the patch against the test-suite and have given a thorough\nread to it, and I think it's fine.\n\nWould you mind sending it as a proper patch ? I have nothing to add,\nand I'm terrible at writing commit messages :-/\nOr maybe someone else's opinion would be nice. I'm still not convinced\nthis is even necessary.\n\nThanks !\n"},{"id":"225878","messageId":"xmqq1u5hkomf.fsf@gitster.dls.corp.google.com","threadId":"34750","inReplyTo":"20130825165153.GC21092@sigill.intra.peff.net","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-26T05:27:52Z","receivedAt":"2013-08-26T05:27:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Exactly. Sample (largely untested) patch is below if you want to use it\n> as a starting point. There are probably a few additional cleanups on top\n> (e.g., \"git log\" understands \"--mailmap\", which should probably be\n> centralized to handle_revision_opt).\n>\n> I'm on the fence. It doesn't actually save that many lines of code, and\n> I guess it's possible that somebody would want a custom mailmap in the\n> future. Even though you can't do it right now, all it would take is\n> exposing read_mailmap_file and read_mailmap_blob outside of mailmap.c.\n> Of course, it would be easy to expose map_user_from at the same time.\n\nI am of two minds on this, but if I were forced to pick one _today_,\nI would have to say that I am moderately negative to the approach.\n\nHaving to always specify that you want to use mailmap and make sure\nyou read it is a bit cumbersome from callers' point of view, and\nusing a singleton global may be one attractive way to do so.\n\nIt however regresses the \"you can choose which mailmap to apply\"\nstructure we already have, it would make things less libifiable, and\nwill make it harder to allow a single Git process work on two or\nmore independent repositories (yes, we would need to restructure the\nobject API to allow us to manage multiple object stores, the ref\nAPI, etc. in a way similar to how we weaned ourselves away from the\nsingle \"active_cache\" abstraction in the index API). I am personally\nOK to declare that we should _never_ touch more than one repository\nin a single process, but submodule support already does this to some\nextent, so...\n\nI think it is a reasonable tentative solution to hook a singleton\ninstance to something that is commonly used, e.g. the rev_info\nstructure, for large subset of commands that do use the structure\nchosen to host that singleton instance, but those that do not work\nbased on revision traversal (e.g. \"grep\") need to also honor mailmap\nconsistently, so we must keep the lower level API that takes an\nexplicit mailmap instance for them anyway.\n\nSo...\n"},{"id":"225941","messageId":"20130826213831.GA6219@sigill.intra.peff.net","threadId":"34750","inReplyTo":"xmqq1u5hkomf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] commit: search author pattern against mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T21:38:31Z","receivedAt":"2013-08-26T21:38:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 25, 2013 at 10:27:52PM -0700, Junio C Hamano wrote:\n\n> > I'm on the fence. It doesn't actually save that many lines of code, and\n> > I guess it's possible that somebody would want a custom mailmap in the\n> > future. Even though you can't do it right now, all it would take is\n> > exposing read_mailmap_file and read_mailmap_blob outside of mailmap.c.\n> > Of course, it would be easy to expose map_user_from at the same time.\n> \n> I am of two minds on this, but if I were forced to pick one _today_,\n> I would have to say that I am moderately negative to the approach.\n> \n> Having to always specify that you want to use mailmap and make sure\n> you read it is a bit cumbersome from callers' point of view, and\n> using a singleton global may be one attractive way to do so.\n\nIt is also slightly wasteful, in that we may parse and store the mailmap\nmultiple times. But I doubt it's a big deal.\n\n> I think it is a reasonable tentative solution to hook a singleton\n> instance to something that is commonly used, e.g. the rev_info\n> structure, for large subset of commands that do use the structure\n> chosen to host that singleton instance, but those that do not work\n> based on revision traversal (e.g. \"grep\") need to also honor mailmap\n> consistently, so we must keep the lower level API that takes an\n> explicit mailmap instance for them anyway.\n\nMy patch kept the lower-level API (well, it de-publicized it because\nnobody was using it, but we do not need to do that part).\n\nBut as I said, I am on the fence, and you do not seem enthused, so let's\njust drop it.\n\n-Peff\n"}]}