{"thread":{"id":"34798","subject":"The gitweb author initials feature from a36817b doesn't work with i18n names","startedAt":"2013-08-29T14:26:29Z","lastAt":"2013-08-31T16:42:36Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Jakub Narębski","Simon Ruderich","Junio C Hamano","Kyle J. McKay"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"226166","messageId":"CACBZZX4i-Tu1G07e9=5DYFjY9EtRGO8gLJf71d5aLeaE-cj-8w@mail.gmail.com","threadId":"34798","inReplyTo":null,"subject":"The gitweb author initials feature from a36817b doesn't work with i18n names","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2013-08-29T14:26:29Z","receivedAt":"2013-08-29T14:26:29Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The @author_initials feature Jakub added in a36817b claims to use a\ni18n regexp (/\\b([[:upper:]])\\B/g), but in Perl this doesn't actually\ndo anything unless the string being matched against has the UTF8 flag.\n\nSo as a result it abbreviates me to \"AB\" not \"ÆAB\". Here's something\nthat demonstrates the issue:\n\n$ cat author-initials.pl\n#!/usr/bin/env perl\nuse strict;\nuse warnings;\n\n#binmode STDOUT, ':utf8';\nopen my $fd, \"-|\", \"git\", \"blame\", \"--incremental\", \"--\", \"Makefile\"\nor die \"Can't open: $!\";\n#binmode $fd, \":utf8\";\nwhile (my $line = <$fd>) {\n next unless my ($author) = $line =~ /^author (.*)/;\n my @author_initials = ($author =~ /\\b([[:upper:]])\\B/g);\n printf \"%s (%s)\\n\", join(\"\", @author_initials), $author;\n}\n\nWith those two binmode commands commented out:\n\n$ perl author-initials.pl |sort|uniq -c|sort -nr|head -n 5\n 99 JH (Junio C Hamano)\n 35 JN (Jonathan Nieder)\n 35 JK (Jeff King)\n 20 JS (Johannes Schindelin)\n 16 AB (Ævar Arnfjörð Bjarmason)\n\nAnd uncommented:\n\n$ perl author-initials.pl |sort|uniq -c|sort -nr|head -n 5\n 99 JH (Junio C Hamano)\n 35 JN (Jonathan Nieder)\n 35 JK (Jeff King)\n 20 JS (Johannes Schindelin)\n 16 ÆAB (Ævar Arnfjörð Bjarmason)\n\nJakub, do you see a reason not to just apply this:\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f429f75..29b3fb5 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6631,6 +6631,7 @@ sub git_blame_common {\n $hash_base, '--', $file_name\n or die_error(500, \"Open git-blame --porcelain failed\");\n }\n+ binmode $fd, \":utf8\";\n\n # incremental blame data returns early\n if ($format eq 'data') {\n\nI haven't gotten an env where I can test gitweb running, but that\nlooks like it should work to me.\n"},{"id":"226167","messageId":"CANQwDwcyV7N6xPxaYFUnRQptVabwkd_mFconZPGDm9iGS3LArA@mail.gmail.com","threadId":"34798","inReplyTo":"CACBZZX4i-Tu1G07e9=5DYFjY9EtRGO8gLJf71d5aLeaE-cj-8w@mail.gmail.com","subject":"Re: The gitweb author initials feature from a36817b doesn't work with i18n names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2013-08-29T14:44:42Z","receivedAt":"2013-08-29T14:44:42Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"[Ævar, sorry for duplication but I accidentally send HTML email; stupid Gmail]\n\nOn Thu, Aug 29, 2013 at 4:26 PM, Ævar Arnfjörð Bjarmason\n wrote:\n\n> The @author_initials feature Jakub added in a36817b claims to use a\n> i18n regexp (/\\b([[:upper:]])\\B/g), but in Perl this doesn't actually\n> do anything unless the string being matched against has the UTF8 flag.\n>\n> So as a result it abbreviates me to \"AB\" not \"ÆAB\".\n[...]\n\nThanks for catching this, and for an analysis.\n\n> Jakub, do you see a reason not to just apply this:\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index f429f75..29b3fb5 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n> $hash_base, '--', $file_name\n> or die_error(500, \"Open git-blame --porcelain failed\");\n> }\n> + binmode $fd, \":utf8\";\n>\n> # incremental blame data returns early\n> if ($format eq 'data') {\n>\n> I haven't gotten an env where I can test gitweb running, but that\n> looks like it should work to me.\n\nUnfortunetly I cannot check this either, but it looks obviously correct.\nACK.\n\n-- \nJakub Narebski\n"},{"id":"226186","messageId":"20130829163935.GA9689@ruderich.org","threadId":"34798","inReplyTo":"CACBZZX4i-Tu1G07e9=5DYFjY9EtRGO8gLJf71d5aLeaE-cj-8w@mail.gmail.com","subject":"Re: The gitweb author initials feature from a36817b doesn't work with i18n names","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2013-08-29T16:39:36Z","receivedAt":"2013-08-29T16:39:36Z","isPatch":false,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Thu, Aug 29, 2013 at 04:26:29PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> I haven't gotten an env where I can test gitweb running, but that\n> looks like it should work to me.\n\nI've tested the patch and it works fine.\n\nTested-by: Simon Ruderich \n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"226333","messageId":"1377851821-5412-1-git-send-email-avarab@gmail.com","threadId":"34798","inReplyTo":"20130829163935.GA9689@ruderich.org","subject":"[PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2013-08-30T08:37:01Z","receivedAt":"2013-08-30T08:37:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the @author_initials feature Jakub added in\nv1.6.4-rc2-14-ga36817b to match non-ASCII author initials as intended.\n\nThe regexp Jakub added was intended to match\nnon-ASCII (/\\b([[:upper:]])\\B/g). But in Perl this doesn't actually\nmatch non-ASCII upper-case characters unless the string being matched\nagainst has the UTF8 flag.\n\nSo when we open a pipe to \"git blame\" we need to mark the file\ndescriptor we're opening as utf8 explicitly.\n\nSo as a result it abbreviates me to \"AB\" not \"ÆAB\", entirely because \"Æ\"\nisn't /[[:upper:]]/ unless the string being matched against has the UTF8\nflag.\n\nHere's something that demonstrates the issue:\n\n #!/usr/bin/env perl\n use strict;\n use warnings;\n\n binmode STDOUT, ':utf8' if $ENV{UTF8};\n open my $fd, \"-|\", \"git\", \"blame\", \"--incremental\", \"--\", \"Makefile\" or die \"Can't open: $!\";\n binmode $fd, \":utf8\" if $ENV{UTF8};\n while (my $line = <$fd>) {\n \tnext unless my ($author) = $line =~ /^author (.*)/;\n \tmy @author_initials = ($author =~ /\\b([[:upper:]])\\B/g);\n \tprintf \"%s (%s)\\n\", join(\"\", @author_initials), $author;\n }\n\nWhen that's run with and without UTF8 being true in the environment it\ngives, on git.git:\n\n $ UTF8=0 perl author-initials.pl | sort | uniq -c |\n sort -nr | head -n 5\n 99 JH (Junio C Hamano)\n 35 JN (Jonathan Nieder)\n 35 JK (Jeff King)\n 20 JS (Johannes Schindelin)\n 16 AB (Ævar Arnfjörð Bjarmason)\n $ UTF8=1 perl author-initials.pl | sort | uniq -c |\n sort -nr | head -n 5\n 99 JH (Junio C Hamano)\n 35 JN (Jonathan Nieder)\n 35 JK (Jeff King)\n 20 JS (Johannes Schindelin)\n 16 ÆAB (Ævar Arnfjörð Bjarmason)\n\nAcked-by: Jakub Narębski \nTested-by: Ævar Arnfjörð Bjarmason \nTested-by: Simon Ruderich \n---\n gitweb/gitweb.perl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f429f75..ad48a5a 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6631,6 +6631,7 @@ sub git_blame_common {\n \t\t\t$hash_base, '--', $file_name\n \t\t\tor die_error(500, \"Open git-blame --porcelain failed\");\n \t}\n+\tbinmode $fh, ':utf8';\n \n \t# incremental blame data returns early\n \tif ($format eq 'data') {\n-- \n1.8.4.rc2\n"},{"id":"226347","messageId":"xmqqppsvyrzj.fsf@gitster.dls.corp.google.com","threadId":"34798","inReplyTo":"1377851821-5412-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-30T18:05:04Z","receivedAt":"2013-08-30T18:05:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Acked-by: Jakub Narębski \n> Tested-by: Ævar Arnfjörð Bjarmason \n> Tested-by: Simon Ruderich \n> ---\n> gitweb/gitweb.perl | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index f429f75..ad48a5a 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n> \t\t\t$hash_base, '--', $file_name\n> \t\t\tor die_error(500, \"Open git-blame --porcelain failed\");\n> \t}\n> +\tbinmode $fh, ':utf8';\n> \n> \t# incremental blame data returns early\n> \tif ($format eq 'data') {\n\n\n[Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol \"$fh\" requires\nexplicit package name at /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl line 6634.\n[Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to compilation errors.\n"},{"id":"226349","messageId":"CANQwDwcyTWgk0jhhGUgP-UZDLQ5EGhh5oueitGD5fpgS5bLtzw@mail.gmail.com","threadId":"34798","inReplyTo":"xmqqppsvyrzj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2013-08-30T18:08:09Z","receivedAt":"2013-08-30T18:08:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, Aug 30, 2013 at 8:05 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n\n>> Acked-by: Jakub Narębski \n>> Tested-by: Ævar Arnfjörð Bjarmason \n>> Tested-by: Simon Ruderich \n>> ---\n>> gitweb/gitweb.perl | 1 +\n>> 1 file changed, 1 insertion(+)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index f429f75..ad48a5a 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n>> $hash_base, '--', $file_name\n>> or die_error(500, \"Open git-blame --porcelain failed\");\n>> }\n>> + binmode $fh, ':utf8';\n\nShould be $fd, not $fh.\n\n> [Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol \"$fh\" requires\n> explicit package name at /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl line 6634.\n> [Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to compilation errors.\n\nI wonder how it passed Ævar and Simon tests. Perhaps proposal had $fd?\n\n-- \nJakub Narebski\n"},{"id":"226350","messageId":"xmqqhae7yrls.fsf@gitster.dls.corp.google.com","threadId":"34798","inReplyTo":"xmqqppsvyrzj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-30T18:13:19Z","receivedAt":"2013-08-30T18:13:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Acked-by: Jakub Narębski \n>> Tested-by: Ævar Arnfjörð Bjarmason \n>> Tested-by: Simon Ruderich \n>> ---\n>> +++ b/gitweb/gitweb.perl\n>> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n>> ...\n>> +\tbinmode $fh, ':utf8';\n\n>\n> [Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol \"$fh\" requires\n> explicit package name at /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl line 6634.\n> [Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to compilation errors.\n\nI think in this function the filehandle is called $fd, not $fh. Has\nany of you really tested this???\n"},{"id":"226360","messageId":"20130830183108.GA4240@ruderich.org","threadId":"34798","inReplyTo":"xmqqhae7yrls.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2013-08-30T18:31:08Z","receivedAt":"2013-08-30T18:31:08Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Fri, Aug 30, 2013 at 11:13:19AM -0700, Junio C Hamano wrote:\n> I think in this function the filehandle is called $fd, not $fh. Has\n> any of you really tested this???\n\nI did, but I applied the change by hand without applying the\npatch directly and didn't notice the difference. Sorry for that.\n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"226371","messageId":"74435A51-B404-4EC8-8CF8-6289F49E6DBD@gmail.com","threadId":"34798","inReplyTo":"xmqqhae7yrls.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-08-30T21:39:53Z","receivedAt":"2013-08-30T21:39:53Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Aug 30, 2013, at 11:13, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> Acked-by: Jakub Narębski \n>>> Tested-by: Ævar Arnfjörð Bjarmason \n>>> Tested-by: Simon Ruderich \n>>> ---\n>>> +++ b/gitweb/gitweb.perl\n>>> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n>>> ...\n>>> +\tbinmode $fh, ':utf8';\n>\n>>\n>> [Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol \"$fh\" requires\n>> explicit package name at /home/gitster/w/buildfarm/next/t/../gitweb/ \n>> gitweb.perl line 6634.\n>> [Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of /home/gitster/ \n>> w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to compilation \n>> errors.\n>\n> I think in this function the filehandle is called $fd, not $fh. Has\n> any of you really tested this???\n\nWhat happens if the author name is written in ISO-8859-1 instead of \nUTF-8 in the actual commit object itself?\n\nI'm pretty sure I've seen this where older commits have a ISO-8859-1 \nauthor name and then newer commits have a UTF-8 version of the same \nauthor's name.\n\nIn fact, in the git repository itself, look at commit 0cb3f80d (UTF-8) \nand commit 7eb93c89 (ISO-8859-1) to see this in action."},{"id":"226420","messageId":"CACBZZX4R2PRRRa=iqXQu_NhvFsTWM5+ktt-x4=sddvB_vptpqA@mail.gmail.com","threadId":"34798","inReplyTo":"xmqqhae7yrls.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2013-08-31T16:29:31Z","receivedAt":"2013-08-31T16:29:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I did. I just clumsily sent out the wrong patch. I.e. tested it\nmanually on another system, and then fat-fingered $fh instead of $fd.\n\nShould I send another patch or do you want to just fix this one up?\n\nOn Fri, Aug 30, 2013 at 8:13 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> Acked-by: Jakub Narębski \n>>> Tested-by: Ævar Arnfjörð Bjarmason \n>>> Tested-by: Simon Ruderich \n>>> ---\n>>> +++ b/gitweb/gitweb.perl\n>>> @@ -6631,6 +6631,7 @@ sub git_blame_common {\n>>> ...\n>>> + binmode $fh, ':utf8';\n>\n>>\n>> [Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol \"$fh\" requires\n>> explicit package name at /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl line 6634.\n>> [Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to compilation errors.\n>\n> I think in this function the filehandle is called $fd, not $fh. Has\n> any of you really tested this???\n"},{"id":"226422","messageId":"CANQwDwdrmyMLZ299t4T-o6uGmfbHL_A83R37_zy0-jyFXEq1+A@mail.gmail.com","threadId":"34798","inReplyTo":"74435A51-B404-4EC8-8CF8-6289F49E6DBD@gmail.com","subject":"Re: [PATCH] gitweb: Fix the author initials in blame for non-ASCII names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2013-08-31T16:42:36Z","receivedAt":"2013-08-31T16:42:36Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, Aug 30, 2013 at 11:39 PM, Kyle J. McKay wrote:\n> On Aug 30, 2013, at 11:13, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>> Ævar Arnfjörð Bjarmason writes:\n\n>>>> + binmode $fh, ':utf8';\n\n> What happens if the author name is written in ISO-8859-1 instead of UTF-8 in\n> the actual commit object itself?\n>\n> I'm pretty sure I've seen this where older commits have a ISO-8859-1 author\n> name and then newer commits have a UTF-8 version of the same author's name.\n>\n> In fact, in the git repository itself, look at commit 0cb3f80d (UTF-8) and\n> commit 7eb93c89 (ISO-8859-1) to see this in action.\n\nWell, then you have a problem, though it is only with old history (before\nintroduction of \"encoding\" header in commit object).\n\nBetter and more complete solution would be to use to_utf8() function\ninstead of 'utf8' layer, which when finding invalid UTF-8 sequence uses\n$fallback_encoding (by default \"latin1\", i.e. ISO-8859-1) instead.\n\n\nIn my TODO list is creating PerlIO layer ':utf8-with-fallback' which would\nreplace all those to_utf8() calls...\n\n-- \nJakub Narebski\n"}]}