"Philip Oakley" <philipoak...@iee.org> writes:

> From: "Junio C Hamano" <gits...@pobox.com>
>> Philip Oakley <philipoak...@iee.org> writes:
>>
>>>  sub handleCompileLine
>>>  {
>>>      my ($line, $lineno) = @_;
>>> -    my @parts = split(' ', $line);
>>> +    # my @parts = split(' ', $line);
>>> +    my @parts = quotewords('\s+', 0, $line);
>>
>> Can somebody enlighten me why/if quotewords is preferrable over
>> shellwords in the context of this patch?
>
> "No" - Ignorance is bliss ;-) I think my cargo culting was the result
> of some googling for "quoting perl variables" or some such, which
> obviously came up with quotewords - I'm happy to take advice on this
> one!
>
> quotewords did appear to work though back when I wrote this: 86dcfcf
> (Properly accept quoted space in filenames, 2012-05-06)

A quick websearch shows me:

  http://cpansearch.perl.org/src/CHORNY/Text-ParseWords-3.29/ParseWords.pm

and comparing the implementations of the two, the difference boils
down to just one line to me.

    sub shellwords {
        my (@lines) = @_;
        my @allwords;

        foreach my $line (@lines) {
            $line =~ s/^\s+//;
            my @words = parse_line('\s+', 0, $line);
            pop @words if (@words and !defined $words[-1]);
            return() unless (@words || !length($line));
            push(@allwords, @words);
        }
        return(@allwords);
    }

In quotewords, the call to parse_line uses $keep not hardcoded 0
(which would not make any difference in the context of your patch),
and it assumes parse_line() never returns a singleton "undef" so the
line "pop ... if @words is a list with 'undef' as its sole element"
is missing.

Of course, the caller would become smaller and sweeter, i.e.

    my @parts = shellwords($line);

I am not familiar with if Perl folks have certain convention to
decide when to use which one, though ;-)




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to