{"thread":{"id":"53951","subject":"[RFC PATCH] gitweb: Map names/emails with mailmap","startedAt":"2020-07-30T04:36:26Z","lastAt":"2020-09-07T22:11:18Z","messageCount":18,"participants":["Emma Brooks","Junio C Hamano","Jeff King","Eric Sunshine","Eric Wong","Joe Perches"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"402455","messageId":"20200730041217.6893-1-me@pluvano.com","threadId":"53951","inReplyTo":null,"subject":"[RFC PATCH] gitweb: Map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-07-30T04:12:17Z","receivedAt":"2020-07-30T04:36:26Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"Add an option to map names and emails to their canonical forms via a\n.mailmap file. This is enabled by default, consistent with the behavior\nof Git itself.\n\nSigned-off-by: Emma Brooks <me@pluvano.com>\n---\n\nThis works, but needs some polish. The read_mailmap code is not\nparticularly clever.\n\n Documentation/gitweb.conf.txt |  5 +++\n gitweb/gitweb.perl            | 79 +++++++++++++++++++++++++++++++++--\n 2 files changed, 80 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba9..2d7551a6a5 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+mailmap::\n+\tUse mailmap to find the canonical name/email for\n+\tcommitters/authors (see linkgit:git-shortlog[1]). Enabled by\n+\tdefault.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782ec..00256704a7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -505,6 +505,12 @@ sub evaluate_uri {\n \t\t'override' => 0,\n \t\t'default' => ['']},\n \n+\t# Enable reading mailmap to determine canonical author\n+\t# information. Enabled by default.\n+\t'mailmap' => {\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n \t# Enable displaying how much time and how many git commands\n \t# it took to generate and display page.  Disabled by default.\n \t# Project specific override is not supported.\n@@ -3490,6 +3496,61 @@ sub parse_tag {\n \treturn %tag\n }\n \n+# Contents of mailmap stored as a referance to a hash with keys in the format\n+# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n+# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n+# already been read.\n+my $mailmap;\n+\n+sub read_mailmap {\n+\tmy %mailmap = ();\n+\topen my $fd, '-|', git_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap'\n+\t\tor die_error(500, 'Failed to read mailmap');\n+\tforeach (split '\\n', <$fd>) {\n+\t\tnext if (/^#/);\n+\t\tif (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t    /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n+\t\t\t# New Name <new@email> <old@email>\n+\t\t\t# New Name <new@email> Old Name <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\t$mailmap{$3}{name} = $1;\n+\t\t\t$mailmap{$3}{email} = $2;\n+\t\t} elsif (/(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t         /(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>)/x) {\n+\t\t\t# New Name <old@email>\n+\t\t\t# <new@email> <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\tif ($1) {\n+\t\t\t\t$mailmap{$3}{email} = $1;\n+\t\t\t} else {\n+\t\t\t\t$mailmap{$3}{name} = $2;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn \\%mailmap;\n+}\n+\n+# Map author name and email based on mailmap. A more specific match\n+# (\"name <email>\") is preferred to a less specific one (\"<email>\").\n+sub map_author {\n+\tmy $name = shift;\n+\tmy $email = shift;\n+\n+\tif (!$mailmap) {\n+\t\t$mailmap = read_mailmap;\n+\t}\n+\n+\tif ($mailmap->{\"$name <$email>\"}) {\n+\t\t$name = $mailmap->{\"$name <$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"$name <$email>\"}{email} || $email;\n+\t} elsif ($mailmap->{\"<$email>\"}) {\n+\t\t$name = $mailmap->{\"<$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"<$email>\"}{email} || $email;\n+\t}\n+\n+\treturn ($name, $email);\n+}\n+\n sub parse_commit_text {\n \tmy ($commit_text, $withparents) = @_;\n \tmy @commit_lines = split '\\n', $commit_text;\n@@ -3517,8 +3578,13 @@ sub parse_commit_text {\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'author_name'}  = $1;\n-\t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'author'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'author_name'}  = $name;\n+\t\t\t\t$co{'author_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3527,8 +3593,13 @@ sub parse_commit_text {\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'committer_name'}  = $1;\n-\t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'committer'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'committer_name'}  = $name;\n+\t\t\t\t$co{'committer_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n"},{"id":"402471","messageId":"xmqqime56kkn.fsf@gitster.c.googlers.com","threadId":"53951","inReplyTo":"20200730041217.6893-1-me@pluvano.com","subject":"Re: [RFC PATCH] gitweb: Map names/emails with mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-30T16:20:40Z","receivedAt":"2020-07-30T16:20:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[jc: Cc'ing Jakub, hoping he's still our resident gitweb expert, as\nan \"RFC\" requests help from experts]\n\nEmma Brooks <me@pluvano.com> writes:\n\n> Add an option to map names and emails to their canonical forms via a\n> .mailmap file. This is enabled by default, consistent with the behavior\n> of Git itself.\n>\n> Signed-off-by: Emma Brooks <me@pluvano.com>\n> ---\n>\n> This works, but needs some polish. The read_mailmap code is not\n> particularly clever.\n>\n>  Documentation/gitweb.conf.txt |  5 +++\n>  gitweb/gitweb.perl            | 79 +++++++++++++++++++++++++++++++++--\n>  2 files changed, 80 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index 7963a79ba9..2d7551a6a5 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n>  CSS stylesheet in `@stylesheets`), it may be appropriate to change\n>  these values.\n>  \n> +mailmap::\n> +\tUse mailmap to find the canonical name/email for\n> +\tcommitters/authors (see linkgit:git-shortlog[1]). Enabled by\n> +\tdefault.\n> +\n>  highlight::\n>  \tServer-side syntax highlight support in \"blob\" view.  It requires\n>  \t`$highlight_bin` program to be available (see the description of\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782ec..00256704a7 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -505,6 +505,12 @@ sub evaluate_uri {\n>  \t\t'override' => 0,\n>  \t\t'default' => ['']},\n>  \n> +\t# Enable reading mailmap to determine canonical author\n> +\t# information. Enabled by default.\n> +\t'mailmap' => {\n> +\t\t'override' => 0,\n> +\t\t'default' => [1]},\n> +\n>  \t# Enable displaying how much time and how many git commands\n>  \t# it took to generate and display page.  Disabled by default.\n>  \t# Project specific override is not supported.\n> @@ -3490,6 +3496,61 @@ sub parse_tag {\n>  \treturn %tag\n>  }\n>  \n> +# Contents of mailmap stored as a referance to a hash with keys in the format\n> +# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n> +# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n> +# already been read.\n> +my $mailmap;\n> +\n> +sub read_mailmap {\n> +\tmy %mailmap = ();\n> +\topen my $fd, '-|', git_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap'\n> +\t\tor die_error(500, 'Failed to read mailmap');\n> +\tforeach (split '\\n', <$fd>) {\n> +\t\tnext if (/^#/);\n> +\t\tif (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n> +\t\t    /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n> +\t\t\t# New Name <new@email> <old@email>\n> +\t\t\t# New Name <new@email> Old Name <old@email>\n> +\t\t\t$mailmap{$3} = ();\n> +\t\t\t$mailmap{$3}{name} = $1;\n> +\t\t\t$mailmap{$3}{email} = $2;\n> +\t\t} elsif (/(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>) (?:\\s+\\#)/x ||\n> +\t\t         /(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>)/x) {\n> +\t\t\t# New Name <old@email>\n> +\t\t\t# <new@email> <old@email>\n> +\t\t\t$mailmap{$3} = ();\n> +\t\t\tif ($1) {\n> +\t\t\t\t$mailmap{$3}{email} = $1;\n> +\t\t\t} else {\n> +\t\t\t\t$mailmap{$3}{name} = $2;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\treturn \\%mailmap;\n> +}\n> +\n> +# Map author name and email based on mailmap. A more specific match\n> +# (\"name <email>\") is preferred to a less specific one (\"<email>\").\n> +sub map_author {\n> +\tmy $name = shift;\n> +\tmy $email = shift;\n> +\n> +\tif (!$mailmap) {\n> +\t\t$mailmap = read_mailmap;\n> +\t}\n> +\n> +\tif ($mailmap->{\"$name <$email>\"}) {\n> +\t\t$name = $mailmap->{\"$name <$email>\"}{name} || $name;\n> +\t\t$email = $mailmap->{\"$name <$email>\"}{email} || $email;\n> +\t} elsif ($mailmap->{\"<$email>\"}) {\n> +\t\t$name = $mailmap->{\"<$email>\"}{name} || $name;\n> +\t\t$email = $mailmap->{\"<$email>\"}{email} || $email;\n> +\t}\n> +\n> +\treturn ($name, $email);\n> +}\n> +\n>  sub parse_commit_text {\n>  \tmy ($commit_text, $withparents) = @_;\n>  \tmy @commit_lines = split '\\n', $commit_text;\n> @@ -3517,8 +3578,13 @@ sub parse_commit_text {\n>  \t\t\t$co{'author_epoch'} = $2;\n>  \t\t\t$co{'author_tz'} = $3;\n>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n> -\t\t\t\t$co{'author_name'}  = $1;\n> -\t\t\t\t$co{'author_email'} = $2;\n> +\t\t\t\tmy ($name, $email) = @_;\n> +\t\t\t\tif (gitweb_check_feature('mailmap')) {\n> +\t\t\t\t\t($name, $email) = map_author($1, $2);\n> +\t\t\t\t\t$co{'author'} = \"$name <$email>\";\n> +\t\t\t\t}\n> +\t\t\t\t$co{'author_name'}  = $name;\n> +\t\t\t\t$co{'author_email'} = $email;\n>  \t\t\t} else {\n>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>  \t\t\t}\n> @@ -3527,8 +3593,13 @@ sub parse_commit_text {\n>  \t\t\t$co{'committer_epoch'} = $2;\n>  \t\t\t$co{'committer_tz'} = $3;\n>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n> -\t\t\t\t$co{'committer_name'}  = $1;\n> -\t\t\t\t$co{'committer_email'} = $2;\n> +\t\t\t\tmy ($name, $email) = @_;\n> +\t\t\t\tif (gitweb_check_feature('mailmap')) {\n> +\t\t\t\t\t($name, $email) = map_author($1, $2);\n> +\t\t\t\t\t$co{'committer'} = \"$name <$email>\";\n> +\t\t\t\t}\n> +\t\t\t\t$co{'committer_name'}  = $name;\n> +\t\t\t\t$co{'committer_email'} = $email;\n>  \t\t\t} else {\n>  \t\t\t\t$co{'committer_name'} = $co{'committer'};\n>  \t\t\t}\n"},{"id":"402513","messageId":"20200731010129.GD240563@coredump.intra.peff.net","threadId":"53951","inReplyTo":"20200730041217.6893-1-me@pluvano.com","subject":"Re: [RFC PATCH] gitweb: Map names/emails with mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T01:01:29Z","receivedAt":"2020-07-31T01:01:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 30, 2020 at 04:12:17AM +0000, Emma Brooks wrote:\n\n> Add an option to map names and emails to their canonical forms via a\n> .mailmap file. This is enabled by default, consistent with the behavior\n> of Git itself.\n\nI'm quite far from an expert in gitweb, but this seems like a good\nfeature to have.\n\nHaving a separate implementation to read and apply mailmaps makes me\nworried that it will behave slightly differently than the C code,\nespecially around corner cases. Is it possible for us to ask git\nprograms that are called by gitweb to do the conversion for us (e.g.,\nby passing \"--use-mailmap\" or using \"%aE\" and \"%aN\" formatters)?\nI won't be surprised if the answer is \"no, we access commits using\nlower-level plumbing\". But it's worth looking into, I think, if you\ndidn't already.\n\n-Peff\n"},{"id":"402518","messageId":"xmqqtuxo4eor.fsf@gitster.c.googlers.com","threadId":"53951","inReplyTo":"20200731010129.GD240563@coredump.intra.peff.net","subject":"Re: [RFC PATCH] gitweb: Map names/emails with mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-31T02:10:44Z","receivedAt":"2020-07-31T02:10: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> On Thu, Jul 30, 2020 at 04:12:17AM +0000, Emma Brooks wrote:\n>\n>> Add an option to map names and emails to their canonical forms via a\n>> .mailmap file. This is enabled by default, consistent with the behavior\n>> of Git itself.\n>\n> I'm quite far from an expert in gitweb, but this seems like a good\n> feature to have.\n>\n> Having a separate implementation to read and apply mailmaps makes me\n> worried that it will behave slightly differently than the C code,\n> especially around corner cases. Is it possible for us to ask git\n> programs that are called by gitweb to do the conversion for us (e.g.,\n> by passing \"--use-mailmap\" or using \"%aE\" and \"%aN\" formatters)?\n> I won't be surprised if the answer is \"no, we access commits using\n> lower-level plumbing\". But it's worth looking into, I think, if you\n> didn't already.\n\nI briefly looked at tweaking \"rev-list --header\" but because it ends\nup calling pretty.c::pp_header() for obvious reasons since we are\ndoing as little processing as possible in CMIT_FMT_RAW format, we do\nnot get to pretty.c::pp_user_info() which is where the mailmap\nconversion happens for the normal \"log\" output.\n\nIt is tempting to split pp_user_info() into two parts (i.e. the\nfirst few lines up to where map_user() is optionally called, and the\nremainder), so that the CMIT_FMT_RAW users can optionally ask for\nmailmap to kick in, but I doubt that it is worth it, if the only\npotential benefitter is gitweb (which I consider is purely\nmaintenance mode---I am surprised the world hasn't yet switched to\ngitiles, cgit and others).\n"},{"id":"403211","messageId":"20200808213457.13116-1-me@pluvano.com","threadId":"53951","inReplyTo":"20200730041217.6893-1-me@pluvano.com","subject":"[PATCH] gitweb: Map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-08-08T21:34:58Z","receivedAt":"2020-08-08T21:37:08Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"Add an option to map names and emails to their canonical forms via a\n.mailmap file. This is enabled by default, consistent with the behavior\nof Git itself.\n\nSigned-off-by: Emma Brooks <me@pluvano.com>\n---\n Documentation/gitweb.conf.txt |  5 +++\n gitweb/gitweb.perl            | 81 +++++++++++++++++++++++++++++++++--\n 2 files changed, 82 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba9..2d7551a6a5 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+mailmap::\n+\tUse mailmap to find the canonical name/email for\n+\tcommitters/authors (see linkgit:git-shortlog[1]). Enabled by\n+\tdefault.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782ec..1ca495b8b4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -505,6 +505,12 @@ sub evaluate_uri {\n \t\t'override' => 0,\n \t\t'default' => ['']},\n \n+\t# Enable reading mailmap to determine canonical author\n+\t# information. Enabled by default.\n+\t'mailmap' => {\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n \t# Enable displaying how much time and how many git commands\n \t# it took to generate and display page.  Disabled by default.\n \t# Project specific override is not supported.\n@@ -3490,6 +3496,63 @@ sub parse_tag {\n \treturn %tag\n }\n \n+# Contents of mailmap stored as a referance to a hash with keys in the format\n+# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n+# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n+# already been read.\n+my $mailmap;\n+\n+sub read_mailmap {\n+\tmy %mailmap = ();\n+\topen my $fd, '-|', quote_command(\n+\t\tgit_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap') . ' 2> /dev/null'\n+\t\tor die_error(500, 'Failed to read mailmap');\n+\treturn \\%mailmap if eof $fd;\n+\tforeach (split '\\n', <$fd>) {\n+\t\tnext if (/^#/);\n+\t\tif (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t    /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n+\t\t\t# New Name <new@email> <old@email>\n+\t\t\t# New Name <new@email> Old Name <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\t($mailmap{$3}{name} = $1) =~ s/^\\s+|\\s+$//g;\n+\t\t\t$mailmap{$3}{email} = $2;\n+\t\t} elsif (/(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t         /(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>)/x) {\n+\t\t\t# New Name <old@email>\n+\t\t\t# <new@email> <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\tif ($1) {\n+\t\t\t\t$mailmap{$3}{email} = $1;\n+\t\t\t} else {\n+\t\t\t\t($mailmap{$3}{name} = $2) =~ s/^\\s+|\\s+$//g;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn \\%mailmap;\n+}\n+\n+# Map author name and email based on mailmap. A more specific match\n+# (\"name <email>\") is preferred to a less specific one (\"<email>\").\n+sub map_author {\n+\tmy $name = shift;\n+\tmy $email = shift;\n+\n+\tif (!$mailmap) {\n+\t\t$mailmap = read_mailmap;\n+\t}\n+\n+\tif ($mailmap->{\"$name <$email>\"}) {\n+\t\t$name = $mailmap->{\"$name <$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"$name <$email>\"}{email} || $email;\n+\t} elsif ($mailmap->{\"<$email>\"}) {\n+\t\t$name = $mailmap->{\"<$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"<$email>\"}{email} || $email;\n+\t}\n+\n+\treturn ($name, $email);\n+}\n+\n sub parse_commit_text {\n \tmy ($commit_text, $withparents) = @_;\n \tmy @commit_lines = split '\\n', $commit_text;\n@@ -3517,8 +3580,13 @@ sub parse_commit_text {\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'author_name'}  = $1;\n-\t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'author'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'author_name'}  = $name;\n+\t\t\t\t$co{'author_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3527,8 +3595,13 @@ sub parse_commit_text {\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'committer_name'}  = $1;\n-\t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'committer'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'committer_name'}  = $name;\n+\t\t\t\t$co{'committer_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n"},{"id":"403242","messageId":"20200809230436.2152-1-me@pluvano.com","threadId":"53951","inReplyTo":"20200808213457.13116-1-me@pluvano.com","subject":"[PATCH v2] gitweb: map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-08-09T23:04:37Z","receivedAt":"2020-08-09T23:06:45Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"Add an option to map names and emails to their canonical forms via a\n.mailmap file. This is enabled by default, consistent with the behavior\nof Git itself.\n\nSigned-off-by: Emma Brooks <me@pluvano.com>\n---\n\nNo code changes. I just fixed a typo in the commit subject (made \"map\"\nlower-case).\n\n Documentation/gitweb.conf.txt |  5 +++\n gitweb/gitweb.perl            | 81 +++++++++++++++++++++++++++++++++--\n 2 files changed, 82 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba9..2d7551a6a5 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+mailmap::\n+\tUse mailmap to find the canonical name/email for\n+\tcommitters/authors (see linkgit:git-shortlog[1]). Enabled by\n+\tdefault.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782ec..1ca495b8b4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -505,6 +505,12 @@ sub evaluate_uri {\n \t\t'override' => 0,\n \t\t'default' => ['']},\n \n+\t# Enable reading mailmap to determine canonical author\n+\t# information. Enabled by default.\n+\t'mailmap' => {\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n+\n \t# Enable displaying how much time and how many git commands\n \t# it took to generate and display page.  Disabled by default.\n \t# Project specific override is not supported.\n@@ -3490,6 +3496,63 @@ sub parse_tag {\n \treturn %tag\n }\n \n+# Contents of mailmap stored as a referance to a hash with keys in the format\n+# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n+# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n+# already been read.\n+my $mailmap;\n+\n+sub read_mailmap {\n+\tmy %mailmap = ();\n+\topen my $fd, '-|', quote_command(\n+\t\tgit_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap') . ' 2> /dev/null'\n+\t\tor die_error(500, 'Failed to read mailmap');\n+\treturn \\%mailmap if eof $fd;\n+\tforeach (split '\\n', <$fd>) {\n+\t\tnext if (/^#/);\n+\t\tif (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t    /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n+\t\t\t# New Name <new@email> <old@email>\n+\t\t\t# New Name <new@email> Old Name <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\t($mailmap{$3}{name} = $1) =~ s/^\\s+|\\s+$//g;\n+\t\t\t$mailmap{$3}{email} = $2;\n+\t\t} elsif (/(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>) (?:\\s+\\#)/x ||\n+\t\t         /(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>)/x) {\n+\t\t\t# New Name <old@email>\n+\t\t\t# <new@email> <old@email>\n+\t\t\t$mailmap{$3} = ();\n+\t\t\tif ($1) {\n+\t\t\t\t$mailmap{$3}{email} = $1;\n+\t\t\t} else {\n+\t\t\t\t($mailmap{$3}{name} = $2) =~ s/^\\s+|\\s+$//g;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn \\%mailmap;\n+}\n+\n+# Map author name and email based on mailmap. A more specific match\n+# (\"name <email>\") is preferred to a less specific one (\"<email>\").\n+sub map_author {\n+\tmy $name = shift;\n+\tmy $email = shift;\n+\n+\tif (!$mailmap) {\n+\t\t$mailmap = read_mailmap;\n+\t}\n+\n+\tif ($mailmap->{\"$name <$email>\"}) {\n+\t\t$name = $mailmap->{\"$name <$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"$name <$email>\"}{email} || $email;\n+\t} elsif ($mailmap->{\"<$email>\"}) {\n+\t\t$name = $mailmap->{\"<$email>\"}{name} || $name;\n+\t\t$email = $mailmap->{\"<$email>\"}{email} || $email;\n+\t}\n+\n+\treturn ($name, $email);\n+}\n+\n sub parse_commit_text {\n \tmy ($commit_text, $withparents) = @_;\n \tmy @commit_lines = split '\\n', $commit_text;\n@@ -3517,8 +3580,13 @@ sub parse_commit_text {\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'author_name'}  = $1;\n-\t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'author'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'author_name'}  = $name;\n+\t\t\t\t$co{'author_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3527,8 +3595,13 @@ sub parse_commit_text {\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'committer_name'}  = $1;\n-\t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tmy ($name, $email) = @_;\n+\t\t\t\tif (gitweb_check_feature('mailmap')) {\n+\t\t\t\t\t($name, $email) = map_author($1, $2);\n+\t\t\t\t\t$co{'committer'} = \"$name <$email>\";\n+\t\t\t\t}\n+\t\t\t\t$co{'committer_name'}  = $name;\n+\t\t\t\t$co{'committer_email'} = $email;\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n"},{"id":"403243","messageId":"CAPig+cT3aMUQapU7i0C3jZaLd2XJwP9SbxFEm_tG_1wj8w1PKw@mail.gmail.com","threadId":"53951","inReplyTo":"20200809230436.2152-1-me@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-10T00:49:59Z","receivedAt":"2020-08-10T00:50:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 9, 2020 at 7:06 PM Emma Brooks <me@pluvano.com> wrote:\n> Add an option to map names and emails to their canonical forms via a\n> .mailmap file. This is enabled by default, consistent with the behavior\n> of Git itself.\n>\n> Signed-off-by: Emma Brooks <me@pluvano.com>\n> ---\n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> @@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n> +mailmap::\n> +       Use mailmap to find the canonical name/email for\n> +       committers/authors (see linkgit:git-shortlog[1]). Enabled by\n> +       default.\n\nIs this setting global or per-repository? (I ask because documentation\nfor other options in this section document whether they can be set\nper-repository.)\n\nShould there be any sort of support for functionality similar to the\n\"mailmap.file\" and \"mailmap.blob\" configuration options in Git itself?\n(Genuine question, not a demand for you to implement such support.)\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> +# Contents of mailmap stored as a referance to a hash with keys in the format\n\ns/referance/reference/\n\n> +# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n> +# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n> +# already been read.\n> +my $mailmap;\n> +\n> +sub read_mailmap {\n> +       my %mailmap = ();\n> +       open my $fd, '-|', quote_command(\n> +               git_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap') . ' 2> /dev/null'\n> +               or die_error(500, 'Failed to read mailmap');\n\nAm I reading this correctly that this will die if the project does not\nhave a .mailmap file? If so, that seems like harsh behavior since\nthere are many projects in the wild lacking a .mailmap file.\n\n> +       return \\%mailmap if eof $fd;\n> +       foreach (split '\\n', <$fd>) {\n\nIf the .mailmap has no content, then the 'foreach' loop won't be\nentered, which means the early 'return' above it is unneeded, correct?\n(Not necessarily asking for the early 'return' to be removed, but more\na case of checking that I'm understanding the logic.)\n\n> +               next if (/^#/);\n> +               if (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n> +                   /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n> +                       # New Name <new@email> <old@email>\n> +                       # New Name <new@email> Old Name <old@email>\n\nThe first regex is intended to handle a trailing \"# comment\", whereas\nthe second regex is for lines lacking a comment, correct? However,\nbecause neither of these expressions are anchored, the second regex\nwill match both types of lines, thus the first regex is redundant. I'm\nguessing, therefore, that your intent was actually to anchor the\nexpressions, perhaps like this:\n\n    if (/^\\s* (.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n        /^\\s* (.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) \\s*$/x) {\n\nAlso, if you're matching lines of the form:\n\n    name1 <email1> [optional-name] <email2>\n\nin which you expect to see \"name1\", then is the loose \"(.*)\\s+\"\ndesirable? Shouldn't it be tighter \"(.+)\\s+\"? For instance:\n\n    if (/^\\s* (.+)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n        /^\\s* (.+)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) \\s*$/x) {\n\n> +                       $mailmap{$3} = ();\n\nI wonder if you should be doing some sort of whitespace normalization\non $3 before using it as a hash key. For instance, if someone has a\n.mailmap that looks like this (where I've used \".\" to represent\nspace):\n\n    name1.<email1>.name2...<email2>\n\nthen $3 will have three spaces between 'name2' and '<email2>' when\nused as a key, and that won't match later when you construct a \"name\n<email>\" key later in map_author() with a single space.\n\n> +                       ($mailmap{$3}{name} = $1) =~ s/^\\s+|\\s+$//g;\n> +                       $mailmap{$3}{email} = $2;\n> +               } elsif (/(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>) (?:\\s+\\#)/x ||\n> +                        /(?: <([^<>]+)>\\s+ | (.+)\\s+ ) (<[^<>]+>)/x) {\n\nSame comment as above about anchoring these patterns...\n\n> +                       # New Name <old@email>\n> +                       # <new@email> <old@email>\n> +                       $mailmap{$3} = ();\n> +                       if ($1) {\n> +                               $mailmap{$3}{email} = $1;\n> +                       } else {\n> +                               ($mailmap{$3}{name} = $2) =~ s/^\\s+|\\s+$//g;\n> +                       }\n> +               }\n> +       }\n> +       return \\%mailmap;\n> +}\n"},{"id":"403244","messageId":"20200810031123.GA3565@pluvano.com","threadId":"53951","inReplyTo":"CAPig+cT3aMUQapU7i0C3jZaLd2XJwP9SbxFEm_tG_1wj8w1PKw@mail.gmail.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-08-10T03:12:00Z","receivedAt":"2020-08-10T03:12:45Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"On 2020-08-09 20:49:59-0400, Eric Sunshine wrote:\n> On Sun, Aug 9, 2020 at 7:06 PM Emma Brooks <me@pluvano.com> wrote:\n> > Add an option to map names and emails to their canonical forms via a\n> > .mailmap file. This is enabled by default, consistent with the behavior\n> > of Git itself.\n> >\n> > Signed-off-by: Emma Brooks <me@pluvano.com>\n> > ---\n> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> > @@ -751,6 +751,11 @@ default font sizes or lineheights are changed (e.g. via adding extra\n> > +mailmap::\n> > +       Use mailmap to find the canonical name/email for\n> > +       committers/authors (see linkgit:git-shortlog[1]). Enabled by\n> > +       default.\n> \n> Is this setting global or per-repository? (I ask because documentation\n> for other options in this section document whether they can be set\n> per-repository.)\n\nGlobal. I'll add a note that it cannot be set per-project, or I could\nadd support for setting it per-project if that's wanted.\n\n> Should there be any sort of support for functionality similar to the\n> \"mailmap.file\" and \"mailmap.blob\" configuration options in Git itself?\n> (Genuine question, not a demand for you to implement such support.)\n\nYes, that would be useful and should probably be supported.\n\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > +# Contents of mailmap stored as a referance to a hash with keys in the format\n> \n> s/referance/reference/\n\nOK.\n\n> > +# of \"name <email>\" or \"<email>\", and values that are hashes containing a\n> > +# replacement \"name\" and/or \"email\". If set (even if empty) the mailmap has\n> > +# already been read.\n> > +my $mailmap;\n> > +\n> > +sub read_mailmap {\n> > +       my %mailmap = ();\n> > +       open my $fd, '-|', quote_command(\n> > +               git_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap') . ' 2> /dev/null'\n> > +               or die_error(500, 'Failed to read mailmap');\n> \n> Am I reading this correctly that this will die if the project does not\n> have a .mailmap file? If so, that seems like harsh behavior since\n> there are many projects in the wild lacking a .mailmap file.\n\nNo, this error message is misleading. The die_error is called if there\nis a problem executing git cat-file, but not if cat-file returns an\nerror. I'll revise this message to be more accurate.\n\n> > +       return \\%mailmap if eof $fd;\n> > +       foreach (split '\\n', <$fd>) {\n> \n> If the .mailmap has no content, then the 'foreach' loop won't be\n> entered, which means the early 'return' above it is unneeded, correct?\n> (Not necessarily asking for the early 'return' to be removed, but more\n> a case of checking that I'm understanding the logic.)\n\nThe early return is intended to catch when there is no mailmap, so $fd\ndoes not get initialized. Without it, you would get an error when you\ntry to split $fd's content:\n\n    Use of uninitialized value $fd in split at [the foreach]\n\n> > +               next if (/^#/);\n> > +               if (/(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n> > +                   /(.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>)/x) {\n> > +                       # New Name <new@email> <old@email>\n> > +                       # New Name <new@email> Old Name <old@email>\n> \n> The first regex is intended to handle a trailing \"# comment\", whereas\n> the second regex is for lines lacking a comment, correct? However,\n> because neither of these expressions are anchored, the second regex\n> will match both types of lines, thus the first regex is redundant. I'm\n> guessing, therefore, that your intent was actually to anchor the\n> expressions, perhaps like this:\n> \n>     if (/^\\s* (.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n>         /^\\s* (.*)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) \\s*$/x) {\n> \n> Also, if you're matching lines of the form:\n> \n>     name1 <email1> [optional-name] <email2>\n> \n> in which you expect to see \"name1\", then is the loose \"(.*)\\s+\"\n> desirable? Shouldn't it be tighter \"(.+)\\s+\"? For instance:\n> \n>     if (/^\\s* (.+)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) (?:\\s+\\#)/x ||\n>         /^\\s* (.+)\\s+ <([^<>]+)>\\s+ ((?:.*\\s+)? <[^<>]+>) \\s*$/x) {\n\nYes and yes. I'll update those.\n\n> > +                       $mailmap{$3} = ();\n> \n> I wonder if you should be doing some sort of whitespace normalization\n> on $3 before using it as a hash key. For instance, if someone has a\n> .mailmap that looks like this (where I've used \".\" to represent\n> space):\n> \n>     name1.<email1>.name2...<email2>\n> \n> then $3 will have three spaces between 'name2' and '<email2>' when\n> used as a key, and that won't match later when you construct a \"name\n> <email>\" key later in map_author() with a single space.\n\nYes, I hadn't considered that case.\n\nThanks.\n"},{"id":"403245","messageId":"CAPig+cT2dFLHp6YhUmD7SeGBy3u1mQMhTTCefRzSfz3sTC0OhA@mail.gmail.com","threadId":"53951","inReplyTo":"20200810031123.GA3565@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-10T05:41:27Z","receivedAt":"2020-08-10T05:41:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 9, 2020 at 11:12 PM Emma Brooks <me@pluvano.com> wrote:\n> On 2020-08-09 20:49:59-0400, Eric Sunshine wrote:\n> > On Sun, Aug 9, 2020 at 7:06 PM Emma Brooks <me@pluvano.com> wrote:\n> > > +mailmap::\n> >\n> > Is this setting global or per-repository? (I ask because documentation\n> > for other options in this section document whether they can be set\n> > per-repository.)\n>\n> Global. I'll add a note that it cannot be set per-project, or I could\n> add support for setting it per-project if that's wanted.\n\nIf it's not much extra work, it might make sense to support\nper-project, if for no other reason, to be consistent with other\nnearby options.\n\n> > Should there be any sort of support for functionality similar to the\n> > \"mailmap.file\" and \"mailmap.blob\" configuration options in Git itself?\n> > (Genuine question, not a demand for you to implement such support.)\n>\n> Yes, that would be useful and should probably be supported.\n\nI don't insist upon it. It can always be added later if someone needs\nit. I was asking about it now because it might have an affect on the\ndesign or type of value which the 'mailmap' option you added above can\naccept, and I was concerned about getting locked into a design without\ntaking these other possibilities into account. For instance, rather\nthan being a simple boolean, perhaps the 'mailmap' option you added\ncould be more expressive, eventually allowing support for an explicit\nfile or blob. This is another reason why I asked if 'mailmap' can be\nper-project, since an explicit mailmap file, and especially a blob,\nwould belong to a particular project. It's just something to think\nabout. (Then again, I'm not a gitweb user, nor am I familiar with its\nconfiguration, so take my observations with a grain of salt.)\n\n> > > +       open my $fd, '-|', quote_command(\n> > > +               git_cmd(), 'cat-file', 'blob', 'HEAD:.mailmap') . ' 2> /dev/null'\n> > > +               or die_error(500, 'Failed to read mailmap');\n> >\n> > Am I reading this correctly that this will die if the project does not\n> > have a .mailmap file? If so, that seems like harsh behavior since\n> > there are many projects in the wild lacking a .mailmap file.\n>\n> No, this error message is misleading. The die_error is called if there\n> is a problem executing git cat-file, but not if cat-file returns an\n> error. I'll revise this message to be more accurate.\n\nOkay, that makes sense.\n\n> > > +       return \\%mailmap if eof $fd;\n> > > +       foreach (split '\\n', <$fd>) {\n> >\n> > If the .mailmap has no content, then the 'foreach' loop won't be\n> > entered, which means the early 'return' above it is unneeded, correct?\n> > (Not necessarily asking for the early 'return' to be removed, but more\n> > a case of checking that I'm understanding the logic.)\n>\n> The early return is intended to catch when there is no mailmap, so $fd\n> does not get initialized. Without it, you would get an error when you\n> try to split $fd's content:\n>\n>     Use of uninitialized value $fd in split at [the foreach]\n\nRight. This follows from my misunderstanding what happened if .mailmap\nwas missing.\n"},{"id":"403254","messageId":"20200810100249.GC37030@coredump.intra.peff.net","threadId":"53951","inReplyTo":"20200809230436.2152-1-me@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-10T10:02:49Z","receivedAt":"2020-08-10T10:02:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2020 at 11:04:37PM +0000, Emma Brooks wrote:\n\n> Add an option to map names and emails to their canonical forms via a\n> .mailmap file. This is enabled by default, consistent with the behavior\n> of Git itself.\n> \n> Signed-off-by: Emma Brooks <me@pluvano.com>\n> ---\n> \n> No code changes. I just fixed a typo in the commit subject (made \"map\"\n> lower-case).\n\nThere was a little discussion in response to v1 on whether we could\nreuse the existing C mailmap code:\n\n  https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n\nDid you have any thoughts on that?\n\n-Peff\n"},{"id":"403327","messageId":"20200811041728.GA1748@pluvano.com","threadId":"53951","inReplyTo":"20200810100249.GC37030@coredump.intra.peff.net","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-08-11T04:17:28Z","receivedAt":"2020-08-11T04:18:07Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> There was a little discussion in response to v1 on whether we could\n> reuse the existing C mailmap code:\n> \n>   https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n> \n> Did you have any thoughts on that?\n\nI think it's probably not worth the effort to make the necessary changes\nto \"rev-list --header\" Junio mentioned, just for gitweb.\n\nI agree it's a bit worrisome to have a second parser that could\npotentially behave slightly differently than the main implementation.\nWhat if we added tests for gitweb's mailmap parsing based on the same\ncases used for Git itself?\n"},{"id":"403329","messageId":"CAPig+cREqPM-hpbvRUxrZwiYRQ+k6Ca=FtY6vufy_Mb45jU9Og@mail.gmail.com","threadId":"53951","inReplyTo":"20200811041728.GA1748@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-11T04:48:53Z","receivedAt":"2020-08-11T04:49:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 11, 2020 at 12:17 AM Emma Brooks <me@pluvano.com> wrote:\n> On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> > There was a little discussion in response to v1 on whether we could\n> > reuse the existing C mailmap code:\n>\n> I think it's probably not worth the effort to make the necessary changes\n> to \"rev-list --header\" Junio mentioned, just for gitweb.\n>\n> I agree it's a bit worrisome to have a second parser that could\n> potentially behave slightly differently than the main implementation.\n> What if we added tests for gitweb's mailmap parsing based on the same\n> cases used for Git itself?\n\nAnother option which people probably won't like is to have gitweb\nstart \"git check-mailmap --stdin\" in the background, leave it running,\nand just feed it author/commit info as needed and read back its\nreplies. The benefit is that you get the .mailmap parsing and\nresolution built into Git itself without needing any extra\nparsing/resolution Perl code or tests. The downside is that people\nmight balk at an extra process hanging around for the duration of\ngitweb itself. (You could also start up \"git check-mailmap\" repeatedly\non-demand, but that would probably be too slow and resource intensive\nfor real-world use.)\n"},{"id":"403330","messageId":"20200811045509.GA81227@coredump.intra.peff.net","threadId":"53951","inReplyTo":"20200811041728.GA1748@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-11T04:55:09Z","receivedAt":"2020-08-11T04:55:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 11, 2020 at 04:17:28AM +0000, Emma Brooks wrote:\n\n> On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> > There was a little discussion in response to v1 on whether we could\n> > reuse the existing C mailmap code:\n> > \n> >   https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n> > \n> > Did you have any thoughts on that?\n> \n> I think it's probably not worth the effort to make the necessary changes\n> to \"rev-list --header\" Junio mentioned, just for gitweb.\n\nYeah, I agree that probably doesn't make sense to change \"rev-list\n--header\". I wonder if git could be using \"rev-list --format\" instead,\nthough, and asking for the specific things it wants. That could improve\nmore than just this case, too (e.g., the C code would be parsing and\nnormalizing author/committer idents, which could make handling of badly\nformatted ones more consistent with other Git tools).\n\nIt may be a big change, though. I don't know the gitweb code very well.\n\n> I agree it's a bit worrisome to have a second parser that could\n> potentially behave slightly differently than the main implementation.\n> What if we added tests for gitweb's mailmap parsing based on the same\n> cases used for Git itself?\n\nThat would certainly help, though I don't know how easy it would be to\nreplicate all of the tests in a maintainable way.\n\n-Peff\n"},{"id":"403335","messageId":"20200811061729.GA7134@dcvr","threadId":"53951","inReplyTo":"20200811041728.GA1748@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-08-11T06:17:29Z","receivedAt":"2020-08-11T06:17:31Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Emma Brooks <me@pluvano.com> wrote:\n> On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> > There was a little discussion in response to v1 on whether we could\n> > reuse the existing C mailmap code:\n> > \n> >   https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n> > \n> > Did you have any thoughts on that?\n> \n> I think it's probably not worth the effort to make the necessary changes\n> to \"rev-list --header\" Junio mentioned, just for gitweb.\n> \n> I agree it's a bit worrisome to have a second parser that could\n> potentially behave slightly differently than the main implementation.\n\n+Cc Joe Perches\n\nFwiw, there's already a GPL-2.0 Perl .mailmap parser in\nscripts/get_maintainer.pl of the Linux kernel which Joe\nmaintains:\n\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/get_maintainer.pl\n\nBeen thinking about adding mailmap support to public-inbox in\nthe send-email reply instructions, too.  (but public-inbox is\nAGPL-3+, so I can't steal the code w/o permission)\n\n> What if we added tests for gitweb's mailmap parsing based on the same\n> cases used for Git itself?\n\nThat's probably fine IMHO; especially if it's just for gitweb display\n(and not writing anything that's meant to be stored forever).\n\nThere's already dozens of different parsers for email addresses,\nMIME, mailbox formats, etc. all with slightly different edge cases;\nthings still mostly work well enough to not be a huge problem.\n(Same goes for Markdown, HTML, formats and even JSON :x)\n"},{"id":"403336","messageId":"c2c4f7106f400260ca7ee2ba709fa43c2f0072c9.camel@perches.com","threadId":"53951","inReplyTo":"20200811061729.GA7134@dcvr","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2020-08-11T06:33:55Z","receivedAt":"2020-08-11T06:40:51Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Tue, 2020-08-11 at 06:17 +0000, Eric Wong wrote:\n> Emma Brooks <me@pluvano.com> wrote:\n> > On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> > > There was a little discussion in response to v1 on whether we could\n> > > reuse the existing C mailmap code:\n> > > \n> > >   https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n> > > \n> > > Did you have any thoughts on that?\n> > \n> > I think it's probably not worth the effort to make the necessary changes\n> > to \"rev-list --header\" Junio mentioned, just for gitweb.\n> > \n> > I agree it's a bit worrisome to have a second parser that could\n> > potentially behave slightly differently than the main implementation.\n> \n> +Cc Joe Perches\n> \n> Fwiw, there's already a GPL-2.0 Perl .mailmap parser in\n> scripts/get_maintainer.pl of the Linux kernel which Joe\n> maintains:\n> \n> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/get_maintainer.pl\n\n+cc Florian Mickler\n\nMight be different behavior, dunno.\n\nFlorian Mickler wrote most of that and I\nbelieve I rewrote it a bit, mostly for style.\n\nIf the perl code is useful to you, do what\nyou will with it, I give you my permission.\n\nI don't believe get_maintainer needs to be\nchanged unless it's shown to be different\nthan what git does already.  I think it's\nthe same output.\n\n> Been thinking about adding mailmap support to public-inbox in\n> the send-email reply instructions, too.  (but public-inbox is\n> AGPL-3+, so I can't steal the code w/o permission)\n> \n> > What if we added tests for gitweb's mailmap parsing based on the same\n> > cases used for Git itself?\n> \n> That's probably fine IMHO; especially if it's just for gitweb display\n> (and not writing anything that's meant to be stored forever).\n> \n> There's already dozens of different parsers for email addresses,\n> MIME, mailbox formats, etc. all with slightly different edge cases;\n> things still mostly work well enough to not be a huge problem.\n> (Same goes for Markdown, HTML, formats and even JSON :x)\n\n"},{"id":"405059","messageId":"20200905025518.GA1524@pluvano.com","threadId":"53951","inReplyTo":"20200811045509.GA81227@coredump.intra.peff.net","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-09-05T02:55:18Z","receivedAt":"2020-09-05T02:55:38Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"On 2020-08-11 00:55:09-0400, Jeff King wrote:\n> On Tue, Aug 11, 2020 at 04:17:28AM +0000, Emma Brooks wrote:\n> \n> > On 2020-08-10 06:02:49-0400, Jeff King wrote:\n> > > There was a little discussion in response to v1 on whether we could\n> > > reuse the existing C mailmap code:\n> > > \n> > >   https://lore.kernel.org/git/20200731010129.GD240563@coredump.intra.peff.net/\n> > > \n> > > Did you have any thoughts on that?\n> > \n> > I think it's probably not worth the effort to make the necessary changes\n> > to \"rev-list --header\" Junio mentioned, just for gitweb.\n> \n> Yeah, I agree that probably doesn't make sense to change \"rev-list\n> --header\". I wonder if git could be using \"rev-list --format\" instead,\n> though, and asking for the specific things it wants. That could improve\n> more than just this case, too (e.g., the C code would be parsing and\n> normalizing author/committer idents, which could make handling of badly\n> formatted ones more consistent with other Git tools).\n> \n> It may be a big change, though. I don't know the gitweb code very well.\n\nThis idea works in my testing, and it should be a small change.\n\nHowever, I couldn't find a way to get \"rev-list --format\" to separate\ncommits with NULs. Is there a way to do this that I'm missing? I was\nable to try the concept in gitweb by switching the \"rev-list --format\"\ncall I would've used to a similar log call, and it seems like a fairly\nsmall change.\n"},{"id":"405060","messageId":"xmqqr1rg7vl8.fsf@gitster.c.googlers.com","threadId":"53951","inReplyTo":"20200905025518.GA1524@pluvano.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-05T03:26:11Z","receivedAt":"2020-09-05T03:26:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emma Brooks <me@pluvano.com> writes:\n\n> However, I couldn't find a way to get \"rev-list --format\" to separate\n> commits with NULs.\n\nA workaround would be \"git rev-list --format='%s%x00'\", iow,\nmanually insert NUL\n\nI would have expected \"-z\" to replace LF with NUL, but that does not\nappear to work X-<.\n\n\n"},{"id":"405141","messageId":"20200907221010.GA1554@pluvano.com","threadId":"53951","inReplyTo":"xmqqr1rg7vl8.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] gitweb: map names/emails with mailmap","fromName":"Emma Brooks","fromEmail":"me@pluvano.com","sentAt":"2020-09-07T22:10:10Z","receivedAt":"2020-09-07T22:11:18Z","isPatch":true,"sender":{"key":"me@pluvano.com","avatar":"https://avatars.githubusercontent.com/u/50312486?v=4"},"body":"On 2020-09-04 20:26:11-0700, Junio C Hamano wrote:\n> Emma Brooks <me@pluvano.com> writes:\n> \n> > However, I couldn't find a way to get \"rev-list --format\" to separate\n> > commits with NULs.\n> \n> A workaround would be \"git rev-list --format='%s%x00'\", iow,\n> manually insert NUL\n> \n> I would have expected \"-z\" to replace LF with NUL, but that does not\n> appear to work X-<.\n\nThanks. I'll need to ignore the extra LF when parsing then. Later, \"-z\"\nsupport could be added/fixed in rev-list (#leftoverbits?) and gitweb\ncould be updated to use that instead.\n"}]}