{"thread":{"id":"7817","subject":"[PATCH] gitweb: use decode_utf8 directly","startedAt":"2007-04-24T14:05:15Z","lastAt":"2007-06-03T22:13:51Z","messageCount":21,"participants":["Ismail Dönmez","Junio C Hamano","Alexandre Julliard","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"40328","messageId":"200704241705.19661.ismail@pardus.org.tr","threadId":"7817","inReplyTo":null,"subject":"[PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-04-24T14:05:15Z","receivedAt":"2007-04-24T14:05:15Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"Hi,\n\ngitweb currently uses Encode::decode function with a wrapper like this :\n\n# very thin wrapper for decode(\"utf8\", $str, Encode::FB_DEFAULT);\nsub to_utf8 {\n       my $str = shift;\n       return decode(\"utf8\", $str, Encode::FB_DEFAULT);\n}\n\nBut for me this gives the following error when I try to view RSS feed for \nLinux kernel GIT repo (local checkout) :\n\nCannot decode string with wide characters \nat /usr/lib/perl5/vendor_perl/5.8.8/i686-linux/Encode.pm line 162.\n\nI Google'd a bit but the relevant information seems to be missing about this \nerror. Anyhow there is no need for a wrapper at all as Encode class has a \ndecode_utf8 function which fixes the problem I am experiencing too and chops \noff the unneeded wrapper.\n\nPatch against git 1.5.1.2 is attached. Comments welcome.\n\nP.S: I am using Encode 2.20 from CPAN which is the latest stable version \navailable.\n\nRegards,\nismail\n\n-- \nLife is a game, and if you aren't in it to win,\nwhat the heck are you still doing here?\n\n-- Linus Torvalds (talking about open source development)\n\n\n--- gitweb/gitweb.perl\t2007-04-24 16:53:00.000000000 +0300\n+++ gitweb/gitweb.perl\t2007-04-24 16:54:22.000000000 +0300\n@@ -566,12 +566,6 @@\n \treturn $input;\n }\n \n-# very thin wrapper for decode(\"utf8\", $str, Encode::FB_DEFAULT);\n-sub to_utf8 {\n-\tmy $str = shift;\n-\treturn decode(\"utf8\", $str, Encode::FB_DEFAULT);\n-}\n-\n # quote unsafe chars, but keep the slash, even when it's not\n # correct, but quoted slashes look too horrible in bookmarks\n sub esc_param {\n@@ -596,7 +590,7 @@\n \tmy $str = shift;\n \tmy %opts = @_;\n \n-\t$str = to_utf8($str);\n+\t$str = decode_utf8($str);\n \t$str = $cgi->escapeHTML($str);\n \tif ($opts{'-nbsp'}) {\n \t\t$str =~ s/ /&nbsp;/g;\n@@ -610,7 +604,7 @@\n \tmy $str = shift;\n \tmy %opts = @_;\n \n-\t$str = to_utf8($str);\n+\t$str = decode_utf8($str);\n \t$str = $cgi->escapeHTML($str);\n \tif ($opts{'-nbsp'}) {\n \t\t$str =~ s/ /&nbsp;/g;\n@@ -893,7 +887,7 @@\n \n \tif (length($short) < length($long)) {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n-\t\t                -title => to_utf8($long)},\n+\t\t                -title => decode_utf8($long)},\n \t\t       esc_html($short) . $extra);\n \t} else {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n@@ -1110,7 +1104,7 @@\n \t\t\tif (check_export_ok(\"$projectroot/$path\")) {\n \t\t\t\tmy $pr = {\n \t\t\t\t\tpath => $path,\n-\t\t\t\t\towner => to_utf8($owner),\n+\t\t\t\t\towner => decode_utf8($owner),\n \t\t\t\t};\n \t\t\t\tpush @list, $pr\n \t\t\t}\n@@ -1139,7 +1133,7 @@\n \t\t\t$pr = unescape($pr);\n \t\t\t$ow = unescape($ow);\n \t\t\tif ($pr eq $project) {\n-\t\t\t\t$owner = to_utf8($ow);\n+\t\t\t\t$owner = decode_utf8($ow);\n \t\t\t\tlast;\n \t\t\t}\n \t\t}\n@@ -1613,7 +1607,7 @@\n \t}\n \tmy $owner = $gcos;\n \t$owner =~ s/[,;].*$//;\n-\treturn to_utf8($owner);\n+\treturn decode_utf8($owner);\n }\n \n ## ......................................................................\n@@ -1696,7 +1690,7 @@\n \n \tmy $title = \"$site_name\";\n \tif (defined $project) {\n-\t\t$title .= \" - \" . to_utf8($project);\n+\t\t$title .= \" - \" . decode_utf8($project);\n \t\tif (defined $action) {\n \t\t\t$title .= \"/$action\";\n \t\t\tif (defined $file_name) {\n@@ -1969,7 +1963,7 @@\n \n \tprint \"<div class=\\\"page_path\\\">\";\n \tprint $cgi->a({-href => href(action=>\"tree\", hash_base=>$hb),\n-\t              -title => 'tree root'}, to_utf8(\"[$project]\"));\n+\t              -title => 'tree root'}, decode_utf8(\"[$project]\"));\n \tprint \" / \";\n \tif (defined $name) {\n \t\tmy @dirname = split '/', $name;\n@@ -2584,7 +2578,7 @@\n \t\t($pr->{'age'}, $pr->{'age_string'}) = @aa;\n \t\tif (!defined $pr->{'descr'}) {\n \t\t\tmy $descr = git_get_project_description($pr->{'path'}) || \"\";\n-\t\t\t$pr->{'descr_long'} = to_utf8($descr);\n+\t\t\t$pr->{'descr_long'} = decode_utf8($descr);\n \t\t\t$pr->{'descr'} = chop_str($descr, 25, 5);\n \t\t}\n \t\tif (!defined $pr->{'owner'}) {\n@@ -3616,7 +3610,7 @@\n \t\t$hash = git_get_head_hash($project);\n \t}\n \n-\tmy $filename = to_utf8(basename($project)) . \"-$hash.tar.$suffix\";\n+\tmy $filename = decode_utf8(basename($project)) . \"-$hash.tar.$suffix\";\n \n \tprint $cgi->header(\n \t\t-type => \"application/$ctype\",\n"},{"id":"40580","messageId":"200704271155.24304.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"200704241705.19661.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-04-27T08:55:16Z","receivedAt":"2007-04-27T08:55:16Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Tuesday 24 April 2007 17:05:15 you wrote:\n> Hi,\n>\n> gitweb currently uses Encode::decode function with a wrapper like this :\n>\n> # very thin wrapper for decode(\"utf8\", $str, Encode::FB_DEFAULT);\n> sub to_utf8 {\n>        my $str = shift;\n>        return decode(\"utf8\", $str, Encode::FB_DEFAULT);\n> }\n>\n> But for me this gives the following error when I try to view RSS feed for\n> Linux kernel GIT repo (local checkout) :\n>\n> Cannot decode string with wide characters\n> at /usr/lib/perl5/vendor_perl/5.8.8/i686-linux/Encode.pm line 162.\n>\n> I Google'd a bit but the relevant information seems to be missing about\n> this error. Anyhow there is no need for a wrapper at all as Encode class\n> has a decode_utf8 function which fixes the problem I am experiencing too\n> and chops off the unneeded wrapper.\n>\n> Patch against git 1.5.1.2 is attached. Comments welcome.\n>\n> P.S: I am using Encode 2.20 from CPAN which is the latest stable version\n> available.\n\nPing? This patch should be harmless and it fixes a real error, can it be \napplied please?\n\nRegards,\nismail\n\n"},{"id":"40584","messageId":"7v1wi6p4lt.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200704271155.24304.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-27T09:07:58Z","receivedAt":"2007-04-27T09:07:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n>> I Google'd a bit but the relevant information seems to be missing about\n>> this error. Anyhow there is no need for a wrapper at all as Encode class\n>> has a decode_utf8 function which fixes the problem I am experiencing too\n>> and chops off the unneeded wrapper.\n>>\n>> Patch against git 1.5.1.2 is attached. Comments welcome.\n>>\n>> P.S: I am using Encode 2.20 from CPAN which is the latest stable version\n>> available.\n>\n> Ping? This patch should be harmless and it fixes a real error, can it be \n> applied please?\n\nI cannot tell if it is harmless.  The original used\n\n\tdecode(\"utf8\", $str, Encode::FB_DEFAULT);\n\nand you made them to:\n\n\tdecode_utf8($str);\n\nAccording to the documentation, decode_utf8($octets [,CHECK])\nshould be equivalent to decode(\"utf8\", $octets [,CHECK]), and\nthe documentation further says that without CHECK, these\nfunctions assume Encode::FB_DEFAULT; in other words, these two\nshould be equivalent.\n\nWhich means that there is something else going on.  Your change\nmay fix what you observed (I do not doubt that it fixed what you\nobserved for you), but without understanding what really is\ngoing on (iow, why it is a fix, when the documentation clearly\nindicates they should be equivalent and it should not fix\nanything), we cannot tell what *ELSE* we are breaking with this\nchange.\n"},{"id":"40589","messageId":"200704271223.03468.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7v1wi6p4lt.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-04-27T09:22:58Z","receivedAt":"2007-04-27T09:22:58Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Friday 27 April 2007 12:07:58 you wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> >> I Google'd a bit but the relevant information seems to be missing about\n> >> this error. Anyhow there is no need for a wrapper at all as Encode class\n> >> has a decode_utf8 function which fixes the problem I am experiencing too\n> >> and chops off the unneeded wrapper.\n> >>\n> >> Patch against git 1.5.1.2 is attached. Comments welcome.\n> >>\n> >> P.S: I am using Encode 2.20 from CPAN which is the latest stable version\n> >> available.\n> >\n> > Ping? This patch should be harmless and it fixes a real error, can it be\n> > applied please?\n>\n> I cannot tell if it is harmless.  The original used\n>\n> \tdecode(\"utf8\", $str, Encode::FB_DEFAULT);\n>\n> and you made them to:\n>\n> \tdecode_utf8($str);\n>\n> According to the documentation, decode_utf8($octets [,CHECK])\n> should be equivalent to decode(\"utf8\", $octets [,CHECK]), and\n> the documentation further says that without CHECK, these\n> functions assume Encode::FB_DEFAULT; in other words, these two\n> should be equivalent.\n>\n> Which means that there is something else going on.  Your change\n> may fix what you observed (I do not doubt that it fixed what you\n> observed for you), but without understanding what really is\n> going on (iow, why it is a fix, when the documentation clearly\n> indicates they should be equivalent and it should not fix\n> anything), we cannot tell what *ELSE* we are breaking with this\n> change.\n\nThat might be a bug in Encode itself indeed, I will dig a bit more. Thanks.\n\nRegards,\nismail\n\n\n\n"},{"id":"40621","messageId":"7vhcr1obuo.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200704271223.03468.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-27T19:29:03Z","receivedAt":"2007-04-27T19:29:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n>> Which means that there is something else going on.  Your change\n>> may fix what you observed (I do not doubt that it fixed what you\n>> observed for you), but without understanding what really is\n>> going on (iow, why it is a fix, when the documentation clearly\n>> indicates they should be equivalent and it should not fix\n>> anything), we cannot tell what *ELSE* we are breaking with this\n>> change.\n>\n> That might be a bug in Encode itself indeed, I will dig a bit more. Thanks.\n\nThanks.\n"},{"id":"40854","messageId":"200705020012.13302.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7vhcr1obuo.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-05-01T21:12:13Z","receivedAt":"2007-05-01T21:12:13Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Friday 27 April 2007 22:29:03 you wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> >> Which means that there is something else going on.  Your change\n> >> may fix what you observed (I do not doubt that it fixed what you\n> >> observed for you), but without understanding what really is\n> >> going on (iow, why it is a fix, when the documentation clearly\n> >> indicates they should be equivalent and it should not fix\n> >> anything), we cannot tell what *ELSE* we are breaking with this\n> >> change.\n> >\n> > That might be a bug in Encode itself indeed, I will dig a bit more.\n> > Thanks.\n>\n> Thanks.\n\nOk found out the reason. decode() tries to decode data that is already UTF-8 \nand borks.\n\nThis is from Encode.pm :\n\nsub decode_utf8($;$) {\n    my ( $str, $check ) = @_;\n    return $str if is_utf8($str); <--- Checks if the $str is already UTF-8\n    if ($check) {\n        return decode( \"utf8\", $str, $check ); <--- Else do what gitweb does\n    [...]\n\nSo my patch is indeed correct. I attach it again for reference. Can it be \nplease applied?\n\nRegards,\nismail\n\n\n--- gitweb/gitweb.perl\t2007-04-24 16:53:00.000000000 +0300\n+++ gitweb/gitweb.perl\t2007-04-24 16:54:22.000000000 +0300\n@@ -566,12 +566,6 @@\n \treturn $input;\n }\n \n-# very thin wrapper for decode(\"utf8\", $str, Encode::FB_DEFAULT);\n-sub to_utf8 {\n-\tmy $str = shift;\n-\treturn decode(\"utf8\", $str, Encode::FB_DEFAULT);\n-}\n-\n # quote unsafe chars, but keep the slash, even when it's not\n # correct, but quoted slashes look too horrible in bookmarks\n sub esc_param {\n@@ -596,7 +590,7 @@\n \tmy $str = shift;\n \tmy %opts = @_;\n \n-\t$str = to_utf8($str);\n+\t$str = decode_utf8($str);\n \t$str = $cgi->escapeHTML($str);\n \tif ($opts{'-nbsp'}) {\n \t\t$str =~ s/ /&nbsp;/g;\n@@ -610,7 +604,7 @@\n \tmy $str = shift;\n \tmy %opts = @_;\n \n-\t$str = to_utf8($str);\n+\t$str = decode_utf8($str);\n \t$str = $cgi->escapeHTML($str);\n \tif ($opts{'-nbsp'}) {\n \t\t$str =~ s/ /&nbsp;/g;\n@@ -893,7 +887,7 @@\n \n \tif (length($short) < length($long)) {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n-\t\t                -title => to_utf8($long)},\n+\t\t                -title => decode_utf8($long)},\n \t\t       esc_html($short) . $extra);\n \t} else {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n@@ -1110,7 +1104,7 @@\n \t\t\tif (check_export_ok(\"$projectroot/$path\")) {\n \t\t\t\tmy $pr = {\n \t\t\t\t\tpath => $path,\n-\t\t\t\t\towner => to_utf8($owner),\n+\t\t\t\t\towner => decode_utf8($owner),\n \t\t\t\t};\n \t\t\t\tpush @list, $pr\n \t\t\t}\n@@ -1139,7 +1133,7 @@\n \t\t\t$pr = unescape($pr);\n \t\t\t$ow = unescape($ow);\n \t\t\tif ($pr eq $project) {\n-\t\t\t\t$owner = to_utf8($ow);\n+\t\t\t\t$owner = decode_utf8($ow);\n \t\t\t\tlast;\n \t\t\t}\n \t\t}\n@@ -1613,7 +1607,7 @@\n \t}\n \tmy $owner = $gcos;\n \t$owner =~ s/[,;].*$//;\n-\treturn to_utf8($owner);\n+\treturn decode_utf8($owner);\n }\n \n ## ......................................................................\n@@ -1696,7 +1690,7 @@\n \n \tmy $title = \"$site_name\";\n \tif (defined $project) {\n-\t\t$title .= \" - \" . to_utf8($project);\n+\t\t$title .= \" - \" . decode_utf8($project);\n \t\tif (defined $action) {\n \t\t\t$title .= \"/$action\";\n \t\t\tif (defined $file_name) {\n@@ -1969,7 +1963,7 @@\n \n \tprint \"<div class=\\\"page_path\\\">\";\n \tprint $cgi->a({-href => href(action=>\"tree\", hash_base=>$hb),\n-\t              -title => 'tree root'}, to_utf8(\"[$project]\"));\n+\t              -title => 'tree root'}, decode_utf8(\"[$project]\"));\n \tprint \" / \";\n \tif (defined $name) {\n \t\tmy @dirname = split '/', $name;\n@@ -2584,7 +2578,7 @@\n \t\t($pr->{'age'}, $pr->{'age_string'}) = @aa;\n \t\tif (!defined $pr->{'descr'}) {\n \t\t\tmy $descr = git_get_project_description($pr->{'path'}) || \"\";\n-\t\t\t$pr->{'descr_long'} = to_utf8($descr);\n+\t\t\t$pr->{'descr_long'} = decode_utf8($descr);\n \t\t\t$pr->{'descr'} = chop_str($descr, 25, 5);\n \t\t}\n \t\tif (!defined $pr->{'owner'}) {\n@@ -3616,7 +3610,7 @@\n \t\t$hash = git_get_head_hash($project);\n \t}\n \n-\tmy $filename = to_utf8(basename($project)) . \"-$hash.tar.$suffix\";\n+\tmy $filename = decode_utf8(basename($project)) . \"-$hash.tar.$suffix\";\n \n \tprint $cgi->header(\n \t\t-type => \"application/$ctype\",\n"},{"id":"40855","messageId":"7v8xc85ill.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200705020012.13302.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-01T21:39:34Z","receivedAt":"2007-05-01T21:39:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n> Ok found out the reason. decode() tries to decode data that is already UTF-8 \n> and borks.\n>\n> This is from Encode.pm :\n>\n> sub decode_utf8($;$) {\n>     my ( $str, $check ) = @_;\n>     return $str if is_utf8($str); <--- Checks if the $str is already UTF-8\n>     if ($check) {\n>         return decode( \"utf8\", $str, $check ); <--- Else do what gitweb does\n>     [...]\n>\n> So my patch is indeed correct.\n\nOk, I think that makes it an improvement from the current code,\nso I'd apply.\n\nBut at the same time I wonder why should the callers be feeding\nan already decoded string to to_utf8().  It might be that some\ncallers needs fixing.\n"},{"id":"40857","messageId":"200705020044.47171.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7v8xc85ill.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-05-01T21:44:46Z","receivedAt":"2007-05-01T21:44:46Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Wednesday 02 May 2007 00:39:34 you wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> > Ok found out the reason. decode() tries to decode data that is already\n> > UTF-8 and borks.\n> >\n> > This is from Encode.pm :\n> >\n> > sub decode_utf8($;$) {\n> >     my ( $str, $check ) = @_;\n> >     return $str if is_utf8($str); <--- Checks if the $str is already\n> > UTF-8 if ($check) {\n> >         return decode( \"utf8\", $str, $check ); <--- Else do what gitweb\n> > does [...]\n> >\n> > So my patch is indeed correct.\n>\n> Ok, I think that makes it an improvement from the current code,\n> so I'd apply.\n>\n> But at the same time I wonder why should the callers be feeding\n> an already decoded string to to_utf8().  It might be that some\n> callers needs fixing.\n\nFWIW it was passing my name \"İsmail Dönmez\" based on user info I guess.\n\nRegards,\nismail\n"},{"id":"40858","messageId":"200705020048.53853.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"200705020044.47171.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-05-01T21:48:53Z","receivedAt":"2007-05-01T21:48:53Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Wednesday 02 May 2007 00:44:46 you wrote:\n> On Wednesday 02 May 2007 00:39:34 you wrote:\n> > Ismail Dönmez <ismail@pardus.org.tr> writes:\n> > > Ok found out the reason. decode() tries to decode data that is already\n> > > UTF-8 and borks.\n> > >\n> > > This is from Encode.pm :\n> > >\n> > > sub decode_utf8($;$) {\n> > >     my ( $str, $check ) = @_;\n> > >     return $str if is_utf8($str); <--- Checks if the $str is already\n> > > UTF-8 if ($check) {\n> > >         return decode( \"utf8\", $str, $check ); <--- Else do what gitweb\n> > > does [...]\n> > >\n> > > So my patch is indeed correct.\n> >\n> > Ok, I think that makes it an improvement from the current code,\n> > so I'd apply.\n> >\n> > But at the same time I wonder why should the callers be feeding\n> > an already decoded string to to_utf8().  It might be that some\n> > callers needs fixing.\n>\n> FWIW it was passing my name \"İsmail Dönmez\" based on user info I guess.\n\nI guess its line 1116:\n\n if (check_export_ok(\"$projectroot/$path\")) {\n\tmy $pr = {\n\tpath => $path,\n\towner => to_utf8($owner), <---- Here\n};\n\nMy system is configured for UTF-8 so $owner will be UTF-8 but in some systems \nit might not be so I don't think there is anything to fix here.\n\nRegards,\nismail\n"},{"id":"40973","messageId":"200705032222.37387.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7v8xc85ill.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-05-03T19:22:32Z","receivedAt":"2007-05-03T19:22:32Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"Hi,\nOn Wednesday 02 May 2007 00:39:34 Junio C Hamano wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> > Ok found out the reason. decode() tries to decode data that is already\n> > UTF-8 and borks.\n> >\n> > This is from Encode.pm :\n> >\n> > sub decode_utf8($;$) {\n> >     my ( $str, $check ) = @_;\n> >     return $str if is_utf8($str); <--- Checks if the $str is already\n> > UTF-8 if ($check) {\n> >         return decode( \"utf8\", $str, $check ); <--- Else do what gitweb\n> > does [...]\n> >\n> > So my patch is indeed correct.\n>\n> Ok, I think that makes it an improvement from the current code,\n> so I'd apply.\n>\n> But at the same time I wonder why should the callers be feeding\n> an already decoded string to to_utf8().  It might be that some\n> callers needs fixing.\n\nIs the patch OK do you want more investigation? Asking because its still not \nin git.git.\n\nRegards,\nismail\n"},{"id":"40974","messageId":"7vsladzp29.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200705032222.37387.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-03T19:26:22Z","receivedAt":"2007-05-03T19:26:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n>> But at the same time I wonder why should the callers be feeding\n>> an already decoded string to to_utf8().  It might be that some\n>> callers needs fixing.\n>\n> Is the patch OK do you want more investigation? Asking because its still not \n> in git.git.\n\nI would say that the patch is an improvement from the current\ncode so it should hit 'master'; I was a bit busy lately and then\nam sick, and also we are post -rc1 freeze now and I was being\ncautious, just in case some nacks from more informed parties\narrive late.\n"},{"id":"43740","messageId":"87zm3ju6tg.fsf@wine.dyndns.org","threadId":"7817","inReplyTo":"7vsladzp29.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Alexandre Julliard","fromEmail":"julliard@winehq.org","sentAt":"2007-06-01T13:45:31Z","receivedAt":"2007-06-01T13:45:31Z","isPatch":true,"sender":{"key":"julliard@winehq.org","avatar":null},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> I would say that the patch is an improvement from the current\n> code so it should hit 'master'; I was a bit busy lately and then\n> am sick, and also we are post -rc1 freeze now and I was being\n> cautious, just in case some nacks from more informed parties\n> arrive late.\n\nSorry for the late nack, but it turns out that this patch breaks diff\noutput on the Wine server for files that are not utf-8.\n\nThe cause is apparently that decode_utf8() returns undef for invalid\nsequences instead of substituting a replacement char like\ndecode(\"utf8\") does.\n\nThat may be considered an Encode bug since we are running a fairly old\nversion (1.99, coming with Debian 3.1), but I'd rather not upgrade\nperl on the server. Could the patch be reverted, or done differently?\n\n-- \nAlexandre Julliard\njulliard@winehq.org\n"},{"id":"43741","messageId":"200706011650.10650.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"87zm3ju6tg.fsf@wine.dyndns.org","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-06-01T13:50:09Z","receivedAt":"2007-06-01T13:50:09Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Friday 01 June 2007 16:45:31 Alexandre Julliard wrote:\n> Junio C Hamano <junkio@cox.net> writes:\n> > I would say that the patch is an improvement from the current\n> > code so it should hit 'master'; I was a bit busy lately and then\n> > am sick, and also we are post -rc1 freeze now and I was being\n> > cautious, just in case some nacks from more informed parties\n> > arrive late.\n>\n> Sorry for the late nack, but it turns out that this patch breaks diff\n> output on the Wine server for files that are not utf-8.\n\nIsn't UTF-8 default even for Linux kernel now?\n\n> The cause is apparently that decode_utf8() returns undef for invalid\n> sequences instead of substituting a replacement char like\n> decode(\"utf8\") does.\n>\n> That may be considered an Encode bug since we are running a fairly old\n> version (1.99, coming with Debian 3.1), but I'd rather not upgrade\n> perl on the server. Could the patch be reverted, or done differently?\n\nSorry but thats too old. Of course I am not the maintainer of GIT so its not \nfor me to decide but well as David Woodhouse puts it, please join us in 21st \ncentury and start using UTF-8.\n\n/ismail\n\n-- \nPerfect is the enemy of good\n"},{"id":"43747","messageId":"87ejkvty7k.fsf@wine.dyndns.org","threadId":"7817","inReplyTo":"200706011650.10650.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Alexandre Julliard","fromEmail":"julliard@winehq.org","sentAt":"2007-06-01T16:51:27Z","receivedAt":"2007-06-01T16:51:27Z","isPatch":true,"sender":{"key":"julliard@winehq.org","avatar":null},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n> Sorry but thats too old. Of course I am not the maintainer of GIT so its not \n> for me to decide but well as David Woodhouse puts it, please join us in 21st \n> century and start using UTF-8.\n\nThat's not very helpful. There can be many valid reasons for not using\nutf-8, in our case compatibility with Windows tools is the main\nreason. And even if we were to convert all our files today, it\nwouldn't help when browsing older versions.\n\nI'm not asking gitweb to magically guess the encoding of the files,\nI'm happy with it replacing invalid sequences with some substitution\nchar, like it did before 1.5.2. But now it is deleting whole lines\nfrom the diff, without any indication that something went wrong.\nThat's not an improvement IMNSHO.\n\n-- \nAlexandre Julliard\njulliard@winehq.org\n"},{"id":"43750","messageId":"7vr6ovzcgr.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"87zm3ju6tg.fsf@wine.dyndns.org","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-06-01T19:44:36Z","receivedAt":"2007-06-01T19:44:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandre Julliard <julliard@winehq.org> writes:\n\n> Sorry for the late nack, but it turns out that this patch breaks diff\n> output on the Wine server for files that are not utf-8.\n>\n> The cause is apparently that decode_utf8() returns undef for invalid\n> sequences instead of substituting a replacement char like\n> decode(\"utf8\") does.\n\nThanks for noticing.  Will revert.\n"},{"id":"43751","messageId":"200706012247.57273.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7vr6ovzcgr.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-06-01T19:47:52Z","receivedAt":"2007-06-01T19:47:52Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Friday 01 June 2007 22:44:36 Junio C Hamano wrote:\n> Alexandre Julliard <julliard@winehq.org> writes:\n> > Sorry for the late nack, but it turns out that this patch breaks diff\n> > output on the Wine server for files that are not utf-8.\n> >\n> > The cause is apparently that decode_utf8() returns undef for invalid\n> > sequences instead of substituting a replacement char like\n> > decode(\"utf8\") does.\n>\n> Thanks for noticing.  Will revert.\n\nWhy are reverting a correct bugfix? :( He's at most using outdated software. \n*sigh*\n\n/ismail\n\n-- \nPerfect is the enemy of good\n"},{"id":"43755","messageId":"7vbqfzzbq8.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200706012247.57273.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-06-01T20:00:31Z","receivedAt":"2007-06-01T20:00:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n> On Friday 01 June 2007 22:44:36 Junio C Hamano wrote:\n>> Alexandre Julliard <julliard@winehq.org> writes:\n>> > Sorry for the late nack, but it turns out that this patch breaks diff\n>> > output on the Wine server for files that are not utf-8.\n>> >\n>> > The cause is apparently that decode_utf8() returns undef for invalid\n>> > sequences instead of substituting a replacement char like\n>> > decode(\"utf8\") does.\n>>\n>> Thanks for noticing.  Will revert.\n>\n> Why are reverting a correct bugfix? :( He's at most using outdated software. \n> *sigh*\n\nI would assume that on top of a revert, with an additional\n\n\treturn $str if is_utf8($str);\n\nto to_utf8() you should be able to fix both installations that\nhas old or new Encode.pm?\n"},{"id":"43756","messageId":"200706012308.41335.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7vbqfzzbq8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-06-01T20:08:36Z","receivedAt":"2007-06-01T20:08:36Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Friday 01 June 2007 23:00:31 you wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> > On Friday 01 June 2007 22:44:36 Junio C Hamano wrote:\n> >> Alexandre Julliard <julliard@winehq.org> writes:\n> >> > Sorry for the late nack, but it turns out that this patch breaks diff\n> >> > output on the Wine server for files that are not utf-8.\n> >> >\n> >> > The cause is apparently that decode_utf8() returns undef for invalid\n> >> > sequences instead of substituting a replacement char like\n> >> > decode(\"utf8\") does.\n> >>\n> >> Thanks for noticing.  Will revert.\n> >\n> > Why are reverting a correct bugfix? :( He's at most using outdated\n> > software. *sigh*\n>\n> I would assume that on top of a revert, with an additional\n>\n> \treturn $str if is_utf8($str);\n>\n> to to_utf8() you should be able to fix both installations that\n> has old or new Encode.pm?\n\nI can try the patch if you can send me what you propose. \n\n/ismail\n\n-- \nPerfect is the enemy of good\n"},{"id":"43799","messageId":"200706021022.31812.jnareb@gmail.com","threadId":"7817","inReplyTo":"87zm3ju6tg.fsf@wine.dyndns.org","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-06-02T08:22:31Z","receivedAt":"2007-06-02T08:22:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 1 Jun 2007, Alexandre Julliard wrote:\n \n> The cause is apparently that decode_utf8() returns undef for invalid\n> sequences instead of substituting a replacement char like\n> decode(\"utf8\") does.\n> \n> That may be considered an Encode bug since we are running a fairly old\n> version (1.99, coming with Debian 3.1), but I'd rather not upgrade\n> perl on the server. Could the patch be reverted, or done differently?\n\nCould you put modern (without this decode_utf8 bug) version of Encode.pm\nin the directory with gitweb.cgi, so gitweb uses new local version and\nnot the one that is installed system-wide?\n-- \nJakub Narebski\nPoland\n"},{"id":"43916","messageId":"7vmyzgn14w.fsf@assigned-by-dhcp.cox.net","threadId":"7817","inReplyTo":"200706012308.41335.ismail@pardus.org.tr","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-03T22:06:55Z","receivedAt":"2007-06-03T22:06:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ismail Dönmez <ismail@pardus.org.tr> writes:\n\n> I can try the patch if you can send me what you propose. \n\nDoes the recent one from Jakub work for you?\n"},{"id":"43917","messageId":"200706040113.56055.ismail@pardus.org.tr","threadId":"7817","inReplyTo":"7vmyzgn14w.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: use decode_utf8 directly","fromName":"Ismail Dönmez","fromEmail":"ismail@pardus.org.tr","sentAt":"2007-06-03T22:13:51Z","receivedAt":"2007-06-03T22:13:51Z","isPatch":true,"sender":{"key":"ismail@pardus.org.tr","avatar":null},"body":"On Monday 04 June 2007 01:06:55 Junio C Hamano wrote:\n> Ismail Dönmez <ismail@pardus.org.tr> writes:\n> > I can try the patch if you can send me what you propose.\n>\n> Does the recent one from Jakub work for you?\n\nIt works fine here.\n\nRegards,\nismail\n\n-- \nPerfect is the enemy of good\n"}]}