Re: [PATCH v1] perf/aggregate: tighten option parsing

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 20, 2018, 17:46 UTC
Message-ID: <CAPig+cSDYVGpaV-beNVG57r3YfQ=Ey5zuyHRkk_tf86NNRdY6w@mail.gmail.com>
In-Reply-To: <20180420121041.32558-1-chriscool@tuxfamily.org>

On Fri, Apr 20, 2018 at 8:10 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 21 quoted lines

> 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@tuxfamily.org>
> ---
> 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.