On Fri, Apr 20 2018, Eric Sunshine wrote:
> On Fri, Apr 20, 2018 at 8:10 AM, Christian Couder
> <christian.couder@xxxxxxxxx> wrote:
>> When passing an option '--foo' that it does not recognize, the
>> aggregate.perl script should die with an helpful error message
>> like:
>>
>> unknown option '--foo' at ./aggregate.perl line 80.
>>
>> rather than:
>>
>> fatal: Needed a single revision
>> rev-parse --verify --foo: command returned error: 128
>>
>> While at it let's also prevent something like
>> 'foo--sort-by=regression' to be handled as if
>> '--sort-by=regression' had been used.
>>
>> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> ---
>> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
>> @@ -46,7 +46,7 @@ while (scalar @ARGV) {
>> - if ($arg =~ /--sort-by(?:=(.*))?/) {
>> + if ($arg =~ /^--sort-by(?:=(.*))?$/) {
>
> Makes sense.
>
>> @@ -76,6 +76,9 @@ while (scalar @ARGV) {
>> + if ($arg =~ /^--.+$/) {
>> + die "unknown option '$arg'";
>> + }
>
> Nit: In this expression, the trailing +$ makes the match no tighter
> than the simpler /^--./, so the latter would be good enough.
>
> 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...
