{"thread":{"id":"45769","subject":"[PATCH] t/perf: correctly align non-ASCII descriptions in output","startedAt":"2017-04-21T19:44:46Z","lastAt":"2017-04-21T22:05:07Z","messageCount":6,"participants":["Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"317512","messageId":"20170421194428.1377-1-avarab@gmail.com","threadId":"45769","inReplyTo":null,"subject":"[PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-04-21T19:44:28Z","receivedAt":"2017-04-21T19:44:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the test descriptions from being treated as binary blobs by\nperl to being treated as UTF-8. This ensures that e.g. a test\ndescription like \"æ\" is counted as 1 character, not 2.\n\nI have WIP performance tests for non-ASCII grep patterns on another\ntopic that are affected by this.\n\nNow instead of:\n\n $ ./run p0000-perf-lib-sanity.sh\n [...]\n 0000.4: export a weird var 0.00(0.00+0.00)\n 0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś 0.00(0.00+0.00)\n 0000.7: important variables available in subshells 0.00(0.00+0.00)\n [...]\n\nWe emit:\n\n [...]\n 0000.4: export a weird var 0.00(0.00+0.00)\n 0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś 0.00(0.00+0.00)\n 0000.7: important variables available in subshells 0.00(0.00+0.00)\n [...]\n\nFixes code originally added in 342e9ef2d9 (\"Introduce a performance\ntesting framework\", 2012-02-17).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/perf/aggregate.perl | 3 +++\n t/perf/p0000-perf-lib-sanity.sh | 2 ++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\nindex 924b19dab4..1dbc85b214 100755\n--- a/t/perf/aggregate.perl\n+++ b/t/perf/aggregate.perl\n@@ -88,6 +88,7 @@ for my $t (@tests) {\n sub read_descr {\n \tmy $name = shift;\n \topen my $fh, \"<\", $name or return \"\";\n+\tbinmode $fh, \":utf8\" or die \"PANIC on binmode: $!\";\n \tmy $line = <$fh>;\n \tclose $fh or die \"cannot close $name\";\n \tchomp $line;\n@@ -147,6 +148,8 @@ for my $t (@subtests) {\n my $totalwidth = 3*@dirs+$descrlen;\n $totalwidth += $_ for (@colwidth);\n \n+binmode STDOUT, \":utf8\" or die \"PANIC on binmode: $!\";\n+\n printf \"%-${descrlen}s\", \"Test\";\n for my $i (0..$#dirs) {\n \tmy $d = $dirs[$i];\ndiff --git a/t/perf/p0000-perf-lib-sanity.sh b/t/perf/p0000-perf-lib-sanity.sh\nindex cf8e1efce7..002c21e52a 100755\n--- a/t/perf/p0000-perf-lib-sanity.sh\n+++ b/t/perf/p0000-perf-lib-sanity.sh\n@@ -33,6 +33,8 @@ test_perf 'export a weird var' '\n \ttest_export bar\n '\n \n+test_perf 'éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś' 'true'\n+\n test_expect_success 'test_export works with weird vars' '\n \techo \"$bar\" &&\n \ttest \"$bar\" = \"weird # variable\"\n-- \n2.11.0\n\n"},{"id":"317519","messageId":"20170421204154.c5mvmnccxkxdm5aw@sigill.intra.peff.net","threadId":"45769","inReplyTo":"20170421194428.1377-1-avarab@gmail.com","subject":"Re: [PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-21T20:41:54Z","receivedAt":"2017-04-21T20:42:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 21, 2017 at 07:44:28PM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> Change the test descriptions from being treated as binary blobs by\n> perl to being treated as UTF-8. This ensures that e.g. a test\n> description like \"æ\" is counted as 1 character, not 2.\n> \n> I have WIP performance tests for non-ASCII grep patterns on another\n> topic that are affected by this.\n\nMakes sense. As this is purely about test titles in our project,\nchoosing utf8 as the only encoding is quite sensible.\n\n> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\n> index 924b19dab4..1dbc85b214 100755\n> --- a/t/perf/aggregate.perl\n> +++ b/t/perf/aggregate.perl\n> @@ -88,6 +88,7 @@ for my $t (@tests) {\n> sub read_descr {\n> \tmy $name = shift;\n> \topen my $fh, \"<\", $name or return \"\";\n> +\tbinmode $fh, \":utf8\" or die \"PANIC on binmode: $!\";\n\nI thought there was some \"use\" flag we could set to just make all of our\nhandles utf8. But all I could come up with was stuff like PERLIO and\n\"perl -C\". Using binmode isn't too bad, though (I think you could\njust do it as part of the open, too, but I'm not sure if antique\nversions of perl support that).\n\n-Peff\n"},{"id":"317521","messageId":"CACBZZX4oMFkZ93YxXrByh-jCK-eVxNBj+UgF77zm5Pq1mzf+WQ@mail.gmail.com","threadId":"45769","inReplyTo":"20170421204154.c5mvmnccxkxdm5aw@sigill.intra.peff.net","subject":"Re: [PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-04-21T21:28:42Z","receivedAt":"2017-04-21T21:29:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Apr 21, 2017 at 10:41 PM, Jeff King wrote:\n> On Fri, Apr 21, 2017 at 07:44:28PM +0000, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Change the test descriptions from being treated as binary blobs by\n>> perl to being treated as UTF-8. This ensures that e.g. a test\n>> description like \"æ\" is counted as 1 character, not 2.\n>>\n>> I have WIP performance tests for non-ASCII grep patterns on another\n>> topic that are affected by this.\n>\n> Makes sense. As this is purely about test titles in our project,\n> choosing utf8 as the only encoding is quite sensible.\n\n*Nod*\n\n>> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\n>> index 924b19dab4..1dbc85b214 100755\n>> --- a/t/perf/aggregate.perl\n>> +++ b/t/perf/aggregate.perl\n>> @@ -88,6 +88,7 @@ for my $t (@tests) {\n>> sub read_descr {\n>> my $name = shift;\n>> open my $fh, \"<\", $name or return \"\";\n>> + binmode $fh, \":utf8\" or die \"PANIC on binmode: $!\";\n>\n> I thought there was some \"use\" flag we could set to just make all of our\n> handles utf8. But all I could come up with was stuff like PERLIO and\n> \"perl -C\". Using binmode isn't too bad, though (I think you could\n> just do it as part of the open, too, but I'm not sure if antique\n> versions of perl support that).\n\n[Debugging perl encoding issues is one of the many perks of my dayjob]\n\nUsing binmode like this is about as straightforward as you can get,\nthe former occurrence could be equivalently replaced by:\n\n utf8::decode(my $line = <$fh>);\n\nBut better just to mark the handle as utf8. There's a fancier way to\ndo it as part of the three-arg-open syntax, but I couldn't remember\nwhether all the perl versions we support have it.\n\nAbout the \"use\" flag, you're probably thinking of the confusingly\nnamed \"use utf8\", but that's to set your source code to utf8, not your\nhandles, e.g.:\n\n$ perl -CA -MDevel::Peek -wE 'use utf8; my $日本語 = shift; Dump $日本語' æ\nSV = PV(0x12cc090) at 0x12cded8\n REFCNT = 1\n FLAGS = (PADMY,POK,pPOK,UTF8)\n PV = 0x12de460 \"\\303\\246\"\\0 [UTF8 \"\\x{e6}\"]\n CUR = 2\n LEN = 16\n\nAs you can see people got a bit overexcited about Unicode in the 90s.\n"},{"id":"317522","messageId":"20170421213553.6vqcmnwolwtieqo4@sigill.intra.peff.net","threadId":"45769","inReplyTo":"CACBZZX4oMFkZ93YxXrByh-jCK-eVxNBj+UgF77zm5Pq1mzf+WQ@mail.gmail.com","subject":"Re: [PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-21T21:35:53Z","receivedAt":"2017-04-21T21:36:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 21, 2017 at 11:28:42PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > I thought there was some \"use\" flag we could set to just make all of our\n> > handles utf8. But all I could come up with was stuff like PERLIO and\n> > \"perl -C\". Using binmode isn't too bad, though (I think you could\n> > just do it as part of the open, too, but I'm not sure if antique\n> > versions of perl support that).\n> \n> [Debugging perl encoding issues is one of the many perks of my dayjob]\n> \n> Using binmode like this is about as straightforward as you can get,\n> the former occurrence could be equivalently replaced by:\n> \n> utf8::decode(my $line = <$fh>);\n> \n> But better just to mark the handle as utf8. There's a fancier way to\n> do it as part of the three-arg-open syntax, but I couldn't remember\n> whether all the perl versions we support have it.\n\nYeah, I agree marking the handle is better. binmode is pretty\nstraightforward, but we'd have to remember to manually set it if we add\nany other handles. That's probably not a big deal in this particular\nscript, though, which is pretty short.\n\n> About the \"use\" flag, you're probably thinking of the confusingly\n> named \"use utf8\", but that's to set your source code to utf8, not your\n> handles, e.g.:\n> \n> $ perl -CA -MDevel::Peek -wE 'use utf8; my $日本語 = shift; Dump $日本語' æ\n> SV = PV(0x12cc090) at 0x12cded8\n> REFCNT = 1\n> FLAGS = (PADMY,POK,pPOK,UTF8)\n> PV = 0x12de460 \"\\303\\246\"\\0 [UTF8 \"\\x{e6}\"]\n> CUR = 2\n> LEN = 16\n> \n> As you can see people got a bit overexcited about Unicode in the 90s.\n\nYeah, I know \"use utf8\" doesn't work for that, but I was thinking there\nwas some other trick. Digging...ah, here it is:\n\n use open ':encoding(utf8)'\n\nNo clue how portable that is. For such a small script it may be better\nto just stick with vanilla binmode().\n\n-Peff\n"},{"id":"317524","messageId":"CACBZZX5ERL4ecLL9AvPVKZGXhAS+RQsSSBQEEK7w3yR86A_tMg@mail.gmail.com","threadId":"45769","inReplyTo":"20170421213553.6vqcmnwolwtieqo4@sigill.intra.peff.net","subject":"Re: [PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-04-21T22:02:02Z","receivedAt":"2017-04-21T22:02:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Apr 21, 2017 at 11:35 PM, Jeff King wrote:\n> On Fri, Apr 21, 2017 at 11:28:42PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > I thought there was some \"use\" flag we could set to just make all of our\n>> > handles utf8. But all I could come up with was stuff like PERLIO and\n>> > \"perl -C\". Using binmode isn't too bad, though (I think you could\n>> > just do it as part of the open, too, but I'm not sure if antique\n>> > versions of perl support that).\n>>\n>> [Debugging perl encoding issues is one of the many perks of my dayjob]\n>>\n>> Using binmode like this is about as straightforward as you can get,\n>> the former occurrence could be equivalently replaced by:\n>>\n>> utf8::decode(my $line = <$fh>);\n>>\n>> But better just to mark the handle as utf8. There's a fancier way to\n>> do it as part of the three-arg-open syntax, but I couldn't remember\n>> whether all the perl versions we support have it.\n>\n> Yeah, I agree marking the handle is better. binmode is pretty\n> straightforward, but we'd have to remember to manually set it if we add\n> any other handles. That's probably not a big deal in this particular\n> script, though, which is pretty short.\n>\n>> About the \"use\" flag, you're probably thinking of the confusingly\n>> named \"use utf8\", but that's to set your source code to utf8, not your\n>> handles, e.g.:\n>>\n>> $ perl -CA -MDevel::Peek -wE 'use utf8; my $日本語 = shift; Dump $日本語' æ\n>> SV = PV(0x12cc090) at 0x12cded8\n>> REFCNT = 1\n>> FLAGS = (PADMY,POK,pPOK,UTF8)\n>> PV = 0x12de460 \"\\303\\246\"\\0 [UTF8 \"\\x{e6}\"]\n>> CUR = 2\n>> LEN = 16\n>>\n>> As you can see people got a bit overexcited about Unicode in the 90s.\n>\n> Yeah, I know \"use utf8\" doesn't work for that, but I was thinking there\n> was some other trick. Digging...ah, here it is:\n>\n> use open ':encoding(utf8)'\n>\n> No clue how portable that is. For such a small script it may be better\n> to just stick with vanilla binmode().\n\nYeah that would work, but doesn't work on 5.8.0, which is the lowest\nversion we support.\n"},{"id":"317525","messageId":"20170421220500.7ovz3dqintcyfhb5@sigill.intra.peff.net","threadId":"45769","inReplyTo":"CACBZZX5ERL4ecLL9AvPVKZGXhAS+RQsSSBQEEK7w3yR86A_tMg@mail.gmail.com","subject":"Re: [PATCH] t/perf: correctly align non-ASCII descriptions in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-21T22:05:00Z","receivedAt":"2017-04-21T22:05:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 22, 2017 at 12:02:02AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > Yeah, I know \"use utf8\" doesn't work for that, but I was thinking there\n> > was some other trick. Digging...ah, here it is:\n> >\n> > use open ':encoding(utf8)'\n> >\n> > No clue how portable that is. For such a small script it may be better\n> > to just stick with vanilla binmode().\n> \n> Yeah that would work, but doesn't work on 5.8.0, which is the lowest\n> version we support.\n\nThanks for looking that up.\n\nFor the benefit of the maintainer, I'll summarize: Ævar's original patch\nis fine as-is.\n\n-Peff\n"}]}