Re: [PATCH 01/16] check-non-portable-shell: complain about "test" a/-o instead of &&/||
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Apr 12, 2021, 18:48 UTC
- Message-ID
- <CAPig+cSiwq3KwSLyfNezx77FvLMu0gDjLq2jCKVnLQ-uVXP3Yw@mail.gmail.com>
- In-Reply-To
- <xmqqsg3vcpqv.fsf@gitster.g>
On Mon, Apr 12, 2021 at 2:38 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes: > >> + /\btest\s+-[a-z]\s+.*?\s+-a\s+/ and err '"test A && test B" preferred to "test A -a B"'; > >> + /\btest\s+-[a-z]\s+.*?\s+-o\s+/ and err '"test A || test B" preferred to "test A -o B"'; > > > > These will only match the simplistic forms of `test -X blah` (where > > "-X" is some single letter option), but will miss expressions such as > > `test "$foo" = bar`. Crafting a regex to match more generally would be > > non-trivial, so this simpler match is a reasonable start. Okay. > > Would it be a trivial improvement to do > > 'test', followed by anything other than '&' or '|', and then > followed by '-a' or '-o' > > instead?
That seems plausible and trivial enough. In fact, I think it eliminates a source of false-positives that Ævar's pattern can report, such as:
test whatever && ls -a foo
and
test whatever && foo -o outfile
among others.