Re: [PATCH v1] perf/aggregate: tighten option parsing
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Apr 25, 2018, 12:55 UTC
- Message-ID
- <CAP8UFD1nkNEbbERvyhSGfSLBDNfD8QcmrYNRy_3J4Mrb7z2ekw@mail.gmail.com>
- In-Reply-To
- <xmqqd0ytb4le.fsf@gitster-ct.c.googlers.com>
On Sat, Apr 21, 2018 at 5:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Not necessarily worth a re-roll.
>>
>> Not that it matters in this case, but just as a bit of Perl rx pedantry,
>> yes his is tighter & more correct. You didn't consider how "." interacts
>> with newlines:
>>
>> $ perl -wE 'my @rx = (qr/^--./, qr/^--.+$/, qr/^--./m, qr/^--.+$/m, qr/^--./s, qr/^--.+$/s); for (@rx) { my $s = "--foo\n--bar"; say $_, "\t", ($s =~ $_ ? 1 : 0) }'
>> (?^u:^--.) 1
>> (?^u:^--.+$) 0
>> (?^um:^--.) 1
>> (?^um:^--.+$) 1
>> (?^us:^--.) 1
>> (?^us:^--.+$) 1
>>
>> I don't think it matters here, not like someone will pass \n in options
>> to aggregate.perl...
>
> Hmph, do we want the command not to barf when "--foo\n--bar" is
> given from the command line and we cannot find such an option?
>
> I thought that the location the match under discussion is used does
> want to see a hit with any option looking string that begins with
> double dashes. I would have expected "tigher and hence incorrect",
> in other words.
>
> Somewhat puzzled...I guess it might be better at this point to just "use Getopt::Long;" (along with "Getopt::Long::Configure qw/ pass_through /;") as git send-email does. It might avoid mistakes and subtle discussions like the above.
Thanks for the reviews, Christian.