git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: Junio C Hamano
Message 5 of 5 in “perf/aggregate: tighten option parsing”
  1. perf/aggregate: tighten option parsingChristian Couder, Apr 20, 2018
  2. Eric SunshineApr 20, 2018
  3. Ævar Arnfjörð BjarmasonApr 20, 2018
  4. Junio C HamanoApr 21, 2018
  5. Christian CouderApr 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.