Re: [PATCH] gitweb: merge boolean feature subroutines

From: demerphq <demerphq@gmail.com>
Date: Jan 3, 2009, 16:51 UTC
Message-ID: <9b18b3110901030851t47c03d0ay75fc91b1ef2ed44b@mail.gmail.com>
In-Reply-To: <20090103164024.GA4205@ftbfs.org>

2009/1/3 Matt Kraai <kraai@ftbfs.org>:

Show 27 quoted lines

> 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.

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

> All of the boolean feature values use 0 or 1, so if this should be
> changed, I think it should probably be done as a separate patch.

As you think is best. It is after all a nit, and probably one that is harmless. But I've been bitten by people not using the languages native booleans before, and well, once bitten twice shy.

cheers, Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"