On Tuesday, January 17, 2006, at 03:26AM, Allison Randal <[EMAIL PROTECTED]>
wrote:
>On Dec 27, 2005, at 18:59, Craig A. Berry wrote:
>
>This patch has a few problems that show up as a large number of
>"uninitialized value" warnings during 'make test'. The first problem
>is that it assumes there will always be a captured value, even from
>an optional atom:
>
>> + if( m/(\w+::)?(\w+)/) {
>> + # substitute case-preserved version of name
>> + my $podname = $2;
>> + my $prefix = $1;
>
>I fixed this one by just changing that last line to:
>
>my $prefix = $1 || '';
Clearly the right thing to do. I'm surprised the uninitialized value warnings
have not caused smoke in the last few weeks that my change has been in blead.
>The second problem is that it assumes that all names are supposed to
>be in mixed case.
My thinking came from the opposite direction, namely that we never know for
sure whether the name we get from the filesystem is the actual, case-preserved
name of the module. We can assume (though not guarantee) that when we get a
name in mixed case, then the original case has probably been preserved, so it
seemed safe enough to skip the check in that instance.
>This line of code determines whether the special
>name extraction code is run:
>
>> + if ($name eq lc($name) || $name eq uc($name)) {
>
>But t/search_50survey_inc.t runs Pod::Simple::Search against all of
>@INC. This includes modules like "threads::shared" and
>"warnings::register". For these, $name equals lc($name), but that
>really is the correct name for the module, so the extraction code
>runs even when it shouldn't. I added an "$^O eq 'VMS'" condition to
>the 'if', to limit where the extraction code runs unnecessarily, but
>if you can come up with a more restrictive general condition it would
>be handy.
I was aware that I was doing the extra check for some instances where it would
give me the same name I already had. I don't know of a general condition that
would be more restrictive and still get the job done. Making the check
VMS-only solves the immediate problem, though I was trying to make sure
Pod::Search would also work on directory trees that have been case-levelled by
weird archiving or file transfer practices or on filesystems that reside on FAT
partitions or some as-yet unknown digital gizmo or data crystal that does not
preserve case.
>The third problem is that when the extraction code runs, it clobbers
>$_ here:
>
>> + while (<PODFILE>) {
>
>And this causes problems elsewhere in the module. (Particularly on
>line 202, which is where one set of the "uninitialized value"
>warnings was coming from.) A 'local $_' fixes the problem, but since
>I consider the root of the problem to be excessive use of $_
>throughout the module (a candidate for a later refactor), I switched
>the patch to using 'my $line' instead of $_.
Looks good. We all deserve three whacks on the head with a copy of Perl Best
Practices for excessive use of $_.
>I don't have access to VMS, so I'd appreciate it if you'd check and
>make sure the patch still solves your problem after my changes. You
>can find the pre-release version here:
>
> http://svn.lohutok.net/nam/trunk/perl5/modules/Pod/Simple/lib/Pod/
>Simple/Search.pm
>
>If all is well, I can release 3.04 tomorrow.
I'll take it for a spin and let you know, probably tonight.
