patchgitweb: pass the key to the feature subroutines

> -sub feature_blame {
> -       my ($val) = git_get_project_config('blame', '--bool');
> +sub feature_bool {
> +       my $key = shift;
> +       my ($val) = git_get_project_config($key, '--bool');
>
>        if ($val eq 'true') {
>                return 1;

> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> [...]
> > -sub feature_blame {
> > -       my ($val) = git_get_project_config('blame', '--bool');
> > +sub feature_bool {
> > +       my $key = shift;
> > +       my ($val) = git_get_project_config($key, '--bool');
> >
> >        if ($val eq 'true') {
> >                return 1;
> 
> Maybe that should be:
> 
>            return ($val eq 'true');
> 
> as It is not a good idea to use 0 as a replacement for perls false, as
> the two have different behaviour.
> 
> $perl -wle'my $val=shift; my $x=$val eq "true"; print "<$_>" for $x, 0+$x' false
> <>
> <0>

> On Sat, Jan 03, 2009 at 05:18:44PM +0100, demerphq wrote:
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> [...]
>> > -sub feature_blame {
>> > -       my ($val) = git_get_project_config('blame', '--bool');
>> > +sub feature_bool {
>> > +       my $key = shift;
>> > +       my ($val) = git_get_project_config($key, '--bool');
>> >
>> >        if ($val eq 'true') {
>> >                return 1;
>>
>> Maybe that should be:
>>
>>            return ($val eq 'true');
>>
>> as It is not a good idea to use 0 as a replacement for perls false, as
>> the two have different behaviour.
>>
>> $perl -wle'my $val=shift; my $x=$val eq "true"; print "<$_>" for $x, 0+$x' false
>> <>
>> <0>
>
> I don't think Perl has *a* false value, but rather has multiple values
> that are treated as false, such as undef, zero, and the empty string.
> Personally, I find 0 clearer than the empty string, but that's
> probably just my C bias sneaking in.

> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> > I don't think Perl has *a* false value, but rather has multiple values
> > that are treated as false, such as undef, zero, and the empty string.
> > Personally, I find 0 clearer than the empty string, but that's
> > probably just my C bias sneaking in.
> 
> Yes it definitely does have a false value, PL_sv_no, and a true value,
> PL_sv_yes (although it is much less important).  It is these values
> which are copied to signify true and false in the cases where the
> internals need to, such as for boolean operators that must return
> false, and things like negation and (in)equality checks.
> 
> It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
> and the empty string in the PV (string) slot.
> 
> You can see one example of its behaviour in my previous mail, and can
> see it further here:
> 
> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
> SV = PVNV(0x952eb10) at 0x952b6f0
>   REFCNT = 2147483647
>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
>   IV = 0
>   NV = 0
>   PV = 0x952eae8 ""\0
>   CUR = 0
>   LEN = 4
> 
> Compare that to:
> 
> perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
> SV = IV(0x94d8398) at 0x94bd678
>   REFCNT = 1
>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
>   IV = 0

> On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> > I don't think Perl has *a* false value, but rather has multiple values
>> > that are treated as false, such as undef, zero, and the empty string.
>> > Personally, I find 0 clearer than the empty string, but that's
>> > probably just my C bias sneaking in.
>>
>> Yes it definitely does have a false value, PL_sv_no, and a true value,
>> PL_sv_yes (although it is much less important).  It is these values
>> which are copied to signify true and false in the cases where the
>> internals need to, such as for boolean operators that must return
>> false, and things like negation and (in)equality checks.
>>
>> It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
>> and the empty string in the PV (string) slot.
>>
>> You can see one example of its behaviour in my previous mail, and can
>> see it further here:
>>
>> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
>> SV = PVNV(0x952eb10) at 0x952b6f0
>>   REFCNT = 2147483647
>>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
>>   IV = 0
>>   NV = 0
>>   PV = 0x952eae8 ""\0
>>   CUR = 0
>>   LEN = 4
>>
>> Compare that to:
>>
>> perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
>> SV = IV(0x94d8398) at 0x94bd678
>>   REFCNT = 1
>>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
>>   IV = 0
>
> Wow, I had no idea about this.  Thanks for the information.
>
> It seems like using these values would require obfuscating the code,
> though.  :(  They only seem to be exposed directly via XS.

> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> [...]
>> -sub feature_blame {
>> -       my ($val) = git_get_project_config('blame', '--bool');
>> +sub feature_bool {
>> +       my $key = shift;
>> +       my ($val) = git_get_project_config($key, '--bool');
>>
>>        if ($val eq 'true') {
>>                return 1;
>
> Maybe that should be:
>
>            return ($val eq 'true');
>
> as It is not a good idea to use 0 as a replacement for perls false, as
> the two have different behaviour.

> demerphq <demerphq@gmail.com> writes:
>
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> [...]
>>> -sub feature_blame {
>>> -       my ($val) = git_get_project_config('blame', '--bool');
>>> +sub feature_bool {
>>> +       my $key = shift;
>>> +       my ($val) = git_get_project_config($key, '--bool');
>>>
>>>        if ($val eq 'true') {
>>>                return 1;
>>
>> Maybe that should be:
>>
>>            return ($val eq 'true');
>>
>> as It is not a good idea to use 0 as a replacement for perls false, as
>> the two have different behaviour.
>
> I'd rather want to keep our scripts free of deep Perl magic.  Even if
> there are SVs that are interpreted as false other than 0, that does not
> necessarily mean you have to fear that 0 can sometimes evaluate to true.

> Why execute more opcodes, and return a slightly surprising false, when
> you dont have to?
> 
> Is it really deep perl magic to do:
> 
>   return $val eq 'true';
> 
> instead of
> 
>   return $val eq 'true' ? 1 : 0;
> 
> or the actual use:
> 
>    if ($val eq 'true') {
>       return 1
>    } else {
>       return 0
>    }
>
> Isn't the former superior just on pure minimalism metrics? Theres less
> code to understand, less code to go wrong, and as a bonus it returns a
> true boolean. Isn't that just a win-win-win? I mean most perl
> programmers I know would instantly convert the latter two to the first
> just on the grounds that the first version is the clearest expression
> of the desired intent.

> I agree that what you suggest is better than the alternatives you
> present.  Unfortunately, none of them match the current behavior.
> Here's the current code:
> 
> 	if ($val eq 'true') {
> 		return 1;
> 	} elsif ($val eq 'false') {
> 		return 0;
> 	}
> 
> 	return $_[0];
> 
> Is there a way to use the form you suggest while falling back to the
> default if $val isn't set to 'true' or 'false'?

> Is it really deep perl magic to do:
>
>   return $val eq 'true';
>
> instead of
>
>   return $val eq 'true' ? 1 : 0;