On Aug 24, 2007, at 9:35 PM, James E Keenan wrote:

Will Coleda wrote:
On Aug 23, 2007, at 10:31 PM, [EMAIL PROTECTED] wrote:

Someone actually reads my commit statements in a branch other than trunk! Saints be praised!

First argument to split should be a regex, not a string. Made this correction in three locations, which makes it more consistent with gmake documentation.
This actually changes the behavior of this split (for good or ill):
 From perldoc -f split:
 > As a special case, specifying a PATTERN of space (' ') will
 > split on white space just as "split" with no arguments does.
 > A "split" on "/\s+/" is
 > like a "split(' ')" except that any leading whitespace produces
 > a null first field.

A feature of split so little used that it's the first I've heard of it in 7+ years of Perl!

You make it sound like poor split's fault. =-)

Nonetheless, I still think /\s+/ is the way to go. Here are the three instances where I proposed changes to lib/Parrot/Configure/ Step.pm's use of split:

<snip>

In each, you have absorbed all possible leading whitespace in a \s+ or \s*. So there's no downside to using /s+/.

Fair enough, but:

And there's an upside: It's more forgiving than requiring that the developer type precisely one wordspace characters between arguments.

That's not how split(' ') works. Please re-read the perldoc quote above.

Here's one of the test stanzas from the reconfigure branch's t/ configure/034-step.t that tests one of these gmake expansion statements:

{
    my $tdir = tempdir( CLEANUP => 1 );
    chdir $tdir or croak "Unable to change to temporary directory";
    my $dummy = 'dummy';
open my $IN, '>', $dummy or croak "Unable to open temp file for writing"; my $line = q{$(basename morgan/lefay/abra.ca.dabra src/foo.c hacks)};
    print $IN $line, "\n";
    close $IN or croak "Unable to close temp file";
    ok( genfile(
        $dummy            => 'CFLAGS',
        expand_gmake_syntax => 1,
), "genfile() did transformation of 'make' 'basename' as expected");
    unlink $dummy or croak "Unable to delete file after testing";
    chdir $cwd or croak "Unable to change back to starting directory";
}

That's actually a hard-tab character (or two) between 'src/foo.c' and 'hacks'. This is consistent with the documentation I studied at http://start.it.uts.edu.au/w/doc/solaris/gmake/make_8.html, which I cited in an inline comment in that test file.

Further thoughts?

Just that it already worked, even with the tabs.

Regards.

--
Will "Coke" Coleda
[EMAIL PROTECTED]


Reply via email to