{"thread":{"id":"16976","subject":"[PATCH] gitweb: pass the key to the feature subroutines","startedAt":"2009-01-03T15:31:31Z","lastAt":"2009-01-04T22:07:53Z","messageCount":12,"participants":["Matt Kraai","demerphq","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"99256","messageId":"1230996692-7182-1-git-send-email-kraai@ftbfs.org","threadId":"16976","inReplyTo":null,"subject":"[PATCH] gitweb: merge boolean feature subroutines","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2009-01-03T15:31:31Z","receivedAt":"2009-01-03T15:31:31Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"Signed-off-by: Matt Kraai <kraai@ftbfs.org>\n---\n gitweb/gitweb.perl |   35 ++++++-----------------------------\n 1 files changed, 6 insertions(+), 29 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 99f71b4..4ba2a9b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -203,7 +203,7 @@ our %feature = (\n \t# $feature{'blame'}{'override'} = 1;\n \t# and in project config gitweb.blame = 0|1;\n \t'blame' => {\n-\t\t'sub' => \\&feature_blame,\n+\t\t'sub' => sub { feature_bool('blame', @_) },\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n@@ -241,7 +241,7 @@ our %feature = (\n \t# $feature{'grep'}{'override'} = 1;\n \t# and in project config gitweb.grep = 0|1;\n \t'grep' => {\n-\t\t'sub' => \\&feature_grep,\n+\t\t'sub' => sub { feature_bool('grep', @_) },\n \t\t'override' => 0,\n \t\t'default' => [1]},\n \n@@ -255,7 +255,7 @@ our %feature = (\n \t# $feature{'pickaxe'}{'override'} = 1;\n \t# and in project config gitweb.pickaxe = 0|1;\n \t'pickaxe' => {\n-\t\t'sub' => \\&feature_pickaxe,\n+\t\t'sub' => sub { feature_bool('pickaxe', @_) },\n \t\t'override' => 0,\n \t\t'default' => [1]},\n \n@@ -363,8 +363,9 @@ sub gitweb_check_feature {\n }\n \n \n-sub feature_blame {\n-\tmy ($val) = git_get_project_config('blame', '--bool');\n+sub feature_bool {\n+\tmy $key = shift;\n+\tmy ($val) = git_get_project_config($key, '--bool');\n \n \tif ($val eq 'true') {\n \t\treturn 1;\n@@ -387,30 +388,6 @@ sub feature_snapshot {\n \treturn @fmts;\n }\n \n-sub feature_grep {\n-\tmy ($val) = git_get_project_config('grep', '--bool');\n-\n-\tif ($val eq 'true') {\n-\t\treturn (1);\n-\t} elsif ($val eq 'false') {\n-\t\treturn (0);\n-\t}\n-\n-\treturn ($_[0]);\n-}\n-\n-sub feature_pickaxe {\n-\tmy ($val) = git_get_project_config('pickaxe', '--bool');\n-\n-\tif ($val eq 'true') {\n-\t\treturn (1);\n-\t} elsif ($val eq 'false') {\n-\t\treturn (0);\n-\t}\n-\n-\treturn ($_[0]);\n-}\n-\n # checking HEAD file with -e is fragile if the repository was\n # initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed\n # and then pruned.\n-- \n1.5.6.5\n"},{"id":"99255","messageId":"1230996692-7182-2-git-send-email-kraai@ftbfs.org","threadId":"16976","inReplyTo":"1230996692-7182-1-git-send-email-kraai@ftbfs.org","subject":"[PATCH] gitweb: pass the key to the feature subroutines","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2009-01-03T15:31:32Z","receivedAt":"2009-01-03T15:31:32Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"Signed-off-by: Matt Kraai <kraai@ftbfs.org>\n---\n gitweb/gitweb.perl |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4ba2a9b..2edefda 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -203,7 +203,7 @@ our %feature = (\n \t# $feature{'blame'}{'override'} = 1;\n \t# and in project config gitweb.blame = 0|1;\n \t'blame' => {\n-\t\t'sub' => sub { feature_bool('blame', @_) },\n+\t\t'sub' => \\&feature_bool,\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n@@ -241,7 +241,7 @@ our %feature = (\n \t# $feature{'grep'}{'override'} = 1;\n \t# and in project config gitweb.grep = 0|1;\n \t'grep' => {\n-\t\t'sub' => sub { feature_bool('grep', @_) },\n+\t\t'sub' => \\&feature_bool,\n \t\t'override' => 0,\n \t\t'default' => [1]},\n \n@@ -255,7 +255,7 @@ our %feature = (\n \t# $feature{'pickaxe'}{'override'} = 1;\n \t# and in project config gitweb.pickaxe = 0|1;\n \t'pickaxe' => {\n-\t\t'sub' => sub { feature_bool('pickaxe', @_) },\n+\t\t'sub' => \\&feature_bool,\n \t\t'override' => 0,\n \t\t'default' => [1]},\n \n@@ -344,7 +344,7 @@ sub gitweb_get_feature {\n \t\twarn \"feature $name is not overrideable\";\n \t\treturn @defaults;\n \t}\n-\treturn $sub->(@defaults);\n+\treturn $sub->($name, @defaults);\n }\n \n # A wrapper to check if a given feature is enabled.\n@@ -377,9 +377,9 @@ sub feature_bool {\n }\n \n sub feature_snapshot {\n-\tmy (@fmts) = @_;\n+\tmy ($key, @fmts) = @_;\n \n-\tmy ($val) = git_get_project_config('snapshot');\n+\tmy ($val) = git_get_project_config($key);\n \n \tif ($val) {\n \t\t@fmts = ($val eq 'none' ? () : split /\\s*[,\\s]\\s*/, $val);\n-- \n1.5.6.5\n"},{"id":"99257","messageId":"9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com","threadId":"16976","inReplyTo":"1230996692-7182-1-git-send-email-kraai@ftbfs.org","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2009-01-03T16:18:44Z","receivedAt":"2009-01-03T16:18:44Z","isPatch":true,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n[...]\n> -sub feature_blame {\n> -       my ($val) = git_get_project_config('blame', '--bool');\n> +sub feature_bool {\n> +       my $key = shift;\n> +       my ($val) = git_get_project_config($key, '--bool');\n>\n>        if ($val eq 'true') {\n>                return 1;\n\nMaybe that should be:\n\n           return ($val eq 'true');\n\nas It is not a good idea to use 0 as a replacement for perls false, as\nthe two have different behaviour.\n\n$perl -wle'my $val=shift; my $x=$val eq \"true\"; print \"<$_>\" for $x, 0+$x' false\n<>\n<0>\n\nCheers,\nyves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"99259","messageId":"20090103164024.GA4205@ftbfs.org","threadId":"16976","inReplyTo":"9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2009-01-03T16:40:24Z","receivedAt":"2009-01-03T16:40:24Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Sat, Jan 03, 2009 at 05:18:44PM +0100, demerphq wrote:\n> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n> [...]\n> > -sub feature_blame {\n> > -       my ($val) = git_get_project_config('blame', '--bool');\n> > +sub feature_bool {\n> > +       my $key = shift;\n> > +       my ($val) = git_get_project_config($key, '--bool');\n> >\n> >        if ($val eq 'true') {\n> >                return 1;\n> \n> Maybe that should be:\n> \n>            return ($val eq 'true');\n> \n> as It is not a good idea to use 0 as a replacement for perls false, as\n> the two have different behaviour.\n> \n> $perl -wle'my $val=shift; my $x=$val eq \"true\"; print \"<$_>\" for $x, 0+$x' false\n> <>\n> <0>\n\nI don't think Perl has *a* false value, but rather has multiple values\nthat are treated as false, such as undef, zero, and the empty string.\nPersonally, I find 0 clearer than the empty string, but that's\nprobably just my C bias sneaking in.\n\nAll of the boolean feature values use 0 or 1, so if this should be\nchanged, I think it should probably be done as a separate patch.\n\n-- \nMatt                                                 http://ftbfs.org/\n"},{"id":"99260","messageId":"9b18b3110901030851t47c03d0ay75fc91b1ef2ed44b@mail.gmail.com","threadId":"16976","inReplyTo":"20090103164024.GA4205@ftbfs.org","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2009-01-03T16:51:50Z","receivedAt":"2009-01-03T16:51:50Z","isPatch":true,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n> On Sat, Jan 03, 2009 at 05:18:44PM +0100, demerphq wrote:\n>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n>> [...]\n>> > -sub feature_blame {\n>> > -       my ($val) = git_get_project_config('blame', '--bool');\n>> > +sub feature_bool {\n>> > +       my $key = shift;\n>> > +       my ($val) = git_get_project_config($key, '--bool');\n>> >\n>> >        if ($val eq 'true') {\n>> >                return 1;\n>>\n>> Maybe that should be:\n>>\n>>            return ($val eq 'true');\n>>\n>> as It is not a good idea to use 0 as a replacement for perls false, as\n>> the two have different behaviour.\n>>\n>> $perl -wle'my $val=shift; my $x=$val eq \"true\"; print \"<$_>\" for $x, 0+$x' false\n>> <>\n>> <0>\n>\n> I don't think Perl has *a* false value, but rather has multiple values\n> that are treated as false, such as undef, zero, and the empty string.\n> Personally, I find 0 clearer than the empty string, but that's\n> probably just my C bias sneaking in.\n\nYes it definitely does have a false value, PL_sv_no, and a true value,\nPL_sv_yes (although it is much less important).  It is these values\nwhich are copied to signify true and false in the cases where the\ninternals need to, such as for boolean operators that must return\nfalse, and things like negation and (in)equality checks.\n\nIt is a so called \"dual var\" SvPVNV, with 0 in the NV (numeric) slot\nand the empty string in the PV (string) slot.\n\nYou can see one example of its behaviour in my previous mail, and can\nsee it further here:\n\n$ perl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\")'\nSV = PVNV(0x952eb10) at 0x952b6f0\n  REFCNT = 2147483647\n  FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)\n  IV = 0\n  NV = 0\n  PV = 0x952eae8 \"\"\\0\n  CUR = 0\n  LEN = 4\n\nCompare that to:\n\nperl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\" ? 1 : 0)'\nSV = IV(0x94d8398) at 0x94bd678\n  REFCNT = 1\n  FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)\n  IV = 0\n\n> All of the boolean feature values use 0 or 1, so if this should be\n> changed, I think it should probably be done as a separate patch.\n\nAs you think is best. It is after all a nit, and probably one that is\nharmless. But I've been bitten by people not using the languages\nnative booleans before, and well, once bitten twice shy.\n\ncheers,\nYves\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"99261","messageId":"20090103171333.GB4205@ftbfs.org","threadId":"16976","inReplyTo":"9b18b3110901030851t47c03d0ay75fc91b1ef2ed44b@mail.gmail.com","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2009-01-03T17:13:34Z","receivedAt":"2009-01-03T17:13:34Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:\n> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n> > I don't think Perl has *a* false value, but rather has multiple values\n> > that are treated as false, such as undef, zero, and the empty string.\n> > Personally, I find 0 clearer than the empty string, but that's\n> > probably just my C bias sneaking in.\n> \n> Yes it definitely does have a false value, PL_sv_no, and a true value,\n> PL_sv_yes (although it is much less important).  It is these values\n> which are copied to signify true and false in the cases where the\n> internals need to, such as for boolean operators that must return\n> false, and things like negation and (in)equality checks.\n> \n> It is a so called \"dual var\" SvPVNV, with 0 in the NV (numeric) slot\n> and the empty string in the PV (string) slot.\n> \n> You can see one example of its behaviour in my previous mail, and can\n> see it further here:\n> \n> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\")'\n> SV = PVNV(0x952eb10) at 0x952b6f0\n>   REFCNT = 2147483647\n>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)\n>   IV = 0\n>   NV = 0\n>   PV = 0x952eae8 \"\"\\0\n>   CUR = 0\n>   LEN = 4\n> \n> Compare that to:\n> \n> perl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\" ? 1 : 0)'\n> SV = IV(0x94d8398) at 0x94bd678\n>   REFCNT = 1\n>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)\n>   IV = 0\n\nWow, I had no idea about this.  Thanks for the information.\n\nIt seems like using these values would require obfuscating the code,\nthough.  :(  They only seem to be exposed directly via XS.\n\n-- \nMatt                                                 http://ftbfs.org/\n"},{"id":"99262","messageId":"9b18b3110901030941n3aa04e5ejb34c6f82c11bce4c@mail.gmail.com","threadId":"16976","inReplyTo":"20090103171333.GB4205@ftbfs.org","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2009-01-03T17:41:42Z","receivedAt":"2009-01-03T17:41:42Z","isPatch":true,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n> On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:\n>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n>> > I don't think Perl has *a* false value, but rather has multiple values\n>> > that are treated as false, such as undef, zero, and the empty string.\n>> > Personally, I find 0 clearer than the empty string, but that's\n>> > probably just my C bias sneaking in.\n>>\n>> Yes it definitely does have a false value, PL_sv_no, and a true value,\n>> PL_sv_yes (although it is much less important).  It is these values\n>> which are copied to signify true and false in the cases where the\n>> internals need to, such as for boolean operators that must return\n>> false, and things like negation and (in)equality checks.\n>>\n>> It is a so called \"dual var\" SvPVNV, with 0 in the NV (numeric) slot\n>> and the empty string in the PV (string) slot.\n>>\n>> You can see one example of its behaviour in my previous mail, and can\n>> see it further here:\n>>\n>> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\")'\n>> SV = PVNV(0x952eb10) at 0x952b6f0\n>>   REFCNT = 2147483647\n>>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)\n>>   IV = 0\n>>   NV = 0\n>>   PV = 0x952eae8 \"\"\\0\n>>   CUR = 0\n>>   LEN = 4\n>>\n>> Compare that to:\n>>\n>> perl -MDevel::Peek -e'print Dump(shift @ARGV eq \"true\" ? 1 : 0)'\n>> SV = IV(0x94d8398) at 0x94bd678\n>>   REFCNT = 1\n>>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)\n>>   IV = 0\n>\n> Wow, I had no idea about this.  Thanks for the information.\n>\n> It seems like using these values would require obfuscating the code,\n> though.  :(  They only seem to be exposed directly via XS.\n\nDepend how you look at it. You have access to them via the negation\nand inequality operators. In a way you can think of the 'not' (or  !)\noperator as being the \"inverted boolean constructor\". So for instance\nthe kind of surprising\n\n  my $bool= !!$val;\n\ncan be used to get a copy of the appropriate PL_sv_yes/no. But the\nneed to do is rare. Luckily. :-)\n\nBut yeah, they are not exposed directly at the perl level. There is no\nkeyword to use to generate them like there is for undef/PL_sv_undef,\nalthough I guess it would be fairly easy to expose them in a similar\nfashion via Scalar::Util.\n\ncheers,\nYves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"99281","messageId":"7vvdsv3af6.fsf@gitster.siamese.dyndns.org","threadId":"16976","inReplyTo":"9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-04T05:30:37Z","receivedAt":"2009-01-04T05:30:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq <demerphq@gmail.com> writes:\n\n> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n> [...]\n>> -sub feature_blame {\n>> -       my ($val) = git_get_project_config('blame', '--bool');\n>> +sub feature_bool {\n>> +       my $key = shift;\n>> +       my ($val) = git_get_project_config($key, '--bool');\n>>\n>>        if ($val eq 'true') {\n>>                return 1;\n>\n> Maybe that should be:\n>\n>            return ($val eq 'true');\n>\n> as It is not a good idea to use 0 as a replacement for perls false, as\n> the two have different behaviour.\n\nI'd rather want to keep our scripts free of deep Perl magic.  Even if\nthere are SVs that are interpreted as false other than 0, that does not\nnecessarily mean you have to fear that 0 can sometimes evaluate to true.\n\nAs long as you refrain from doing something crazy like \"0 but true\",\npeople who are not (and/or are not inclined to become) familiar with the\ngory innards of Perl can rely on 0 being false and 1 being true when\ncalling feature_something subs, no?\n"},{"id":"99294","messageId":"9b18b3110901040341n5ff5fa09s878228131d11d2a6@mail.gmail.com","threadId":"16976","inReplyTo":"7vvdsv3af6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2009-01-04T11:41:14Z","receivedAt":"2009-01-04T11:41:14Z","isPatch":true,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"2009/1/4 Junio C Hamano <gitster@pobox.com>:\n> demerphq <demerphq@gmail.com> writes:\n>\n>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:\n>> [...]\n>>> -sub feature_blame {\n>>> -       my ($val) = git_get_project_config('blame', '--bool');\n>>> +sub feature_bool {\n>>> +       my $key = shift;\n>>> +       my ($val) = git_get_project_config($key, '--bool');\n>>>\n>>>        if ($val eq 'true') {\n>>>                return 1;\n>>\n>> Maybe that should be:\n>>\n>>            return ($val eq 'true');\n>>\n>> as It is not a good idea to use 0 as a replacement for perls false, as\n>> the two have different behaviour.\n>\n> I'd rather want to keep our scripts free of deep Perl magic.  Even if\n> there are SVs that are interpreted as false other than 0, that does not\n> necessarily mean you have to fear that 0 can sometimes evaluate to true.\n\nNo, thats not the point. The point is that why should the code do more\nwork to return a value that a perl programmer might find unexpected.\nEspecially when the function has the word \"bool\" in it. Its like\nwriting a function whose name says \"int\" that returns a double. If the\nroutine was not called \"bool\" then it wouldnt matter at all.\n\n> As long as you refrain from doing something crazy like \"0 but true\",\n> people who are not (and/or are not inclined to become) familiar with the\n> gory innards of Perl can rely on 0 being false and 1 being true when\n> calling feature_something subs, no?\n\nWhy execute more opcodes, and return a slightly surprising false, when\nyou dont have to?\n\nIs it really deep perl magic to do:\n\n  return $val eq 'true';\n\ninstead of\n\n  return $val eq 'true' ? 1 : 0;\n\nor the actual use:\n\n   if ($val eq 'true') {\n      return 1\n   } else {\n      return 0\n   }\n\nIsn't the former superior just on pure minimalism metrics? Theres less\ncode to understand, less code to go wrong, and as a bonus it returns a\ntrue boolean. Isn't that just a win-win-win? I mean most perl\nprogrammers I know would instantly convert the latter two to the first\njust on the grounds that the first version is the clearest expression\nof the desired intent.\n\nAnyway, leave it or not, its a minor nit.\n\ncheers,\nYves\n\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"99307","messageId":"20090104155858.GC4205@ftbfs.org","threadId":"16976","inReplyTo":"9b18b3110901040341n5ff5fa09s878228131d11d2a6@mail.gmail.com","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2009-01-04T15:58:58Z","receivedAt":"2009-01-04T15:58:58Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Sun, Jan 04, 2009 at 12:41:14PM +0100, demerphq wrote:\n> Why execute more opcodes, and return a slightly surprising false, when\n> you dont have to?\n> \n> Is it really deep perl magic to do:\n> \n>   return $val eq 'true';\n> \n> instead of\n> \n>   return $val eq 'true' ? 1 : 0;\n> \n> or the actual use:\n> \n>    if ($val eq 'true') {\n>       return 1\n>    } else {\n>       return 0\n>    }\n>\n> Isn't the former superior just on pure minimalism metrics? Theres less\n> code to understand, less code to go wrong, and as a bonus it returns a\n> true boolean. Isn't that just a win-win-win? I mean most perl\n> programmers I know would instantly convert the latter two to the first\n> just on the grounds that the first version is the clearest expression\n> of the desired intent.\n\nI agree that what you suggest is better than the alternatives you\npresent.  Unfortunately, none of them match the current behavior.\nHere's the current code:\n\n\tif ($val eq 'true') {\n\t\treturn 1;\n\t} elsif ($val eq 'false') {\n\t\treturn 0;\n\t}\n\n\treturn $_[0];\n\nIs there a way to use the form you suggest while falling back to the\ndefault if $val isn't set to 'true' or 'false'?\n\n-- \nMatt                                                 http://ftbfs.org/\n"},{"id":"99326","messageId":"7v8wpqu5k1.fsf@gitster.siamese.dyndns.org","threadId":"16976","inReplyTo":"9b18b3110901040341n5ff5fa09s878228131d11d2a6@mail.gmail.com","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-04T21:25:50Z","receivedAt":"2009-01-04T21:25:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq <demerphq@gmail.com> writes:\n\n> Is it really deep perl magic to do:\n>\n>   return $val eq 'true';\n>\n> instead of\n>\n>   return $val eq 'true' ? 1 : 0;\n\nNo, neither are magicky.  But your argument to favor the former over the\nlatter that goes down to XS level was all about deep magic, and you wrote\nyourself:\n\n> ... It is not a good idea to use 0 as a replacement for perls false, as\n> the two have different behaviour.\n\nMy point is that any caller that cares about the differences of \"Perl's\ntrue false\" and 0 when talking about a function that returns a yes/no\nvalue is already soaked too deep in Perl's deep magic.  I would want the\ncode to be maintainable by people who does not care the deep voodoo, and\nfor that reason, I do not want the callers to care.\n\nHaving said that, I think it is perfectly fine to favor returning \"$val eq\n'true'\" over returning \"$val eq 'true ? 1 : 0\".  But that is not because\nit is truer way to say false from Perl experts' point of view, but because\nit is shorter and more to the point.\n"},{"id":"99330","messageId":"m37i5abu8a.fsf@localhost.localdomain","threadId":"16976","inReplyTo":"20090104155858.GC4205@ftbfs.org","subject":"Re: [PATCH] gitweb: merge boolean feature subroutines","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-04T22:07:53Z","receivedAt":"2009-01-04T22:07:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Matt Kraai <kraai@ftbfs.org> writes:\n\n> I agree that what you suggest is better than the alternatives you\n> present.  Unfortunately, none of them match the current behavior.\n> Here's the current code:\n> \n> \tif ($val eq 'true') {\n> \t\treturn 1;\n> \t} elsif ($val eq 'false') {\n> \t\treturn 0;\n> \t}\n> \n> \treturn $_[0];\n> \n> Is there a way to use the form you suggest while falling back to the\n> default if $val isn't set to 'true' or 'false'?\n\nIIRC the return value is actually threestate: 1 for true, 0 for false,\nand undef for non-bool config value.\n\nBTW. git_get_project_config currently emulates old one git-config call\nper configuration variable, with git-config normalizing boolean values\n(returning 'true' or 'false'). But it could return Perl truish or Perl\nfalsish instead, as we now use \"git config -l -z\" + caching config\nvariables, and it is Perl that does normalization of boolean values.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"}]}