On Wed, Apr 21 2021, Đoàn Trần Công Danh wrote:

On 2021-04-21 10:46:08+0200, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

On Wed, Apr 21 2021, Junio C Hamano wrote:

quoted

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

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`. We stop at "&" or "|" to try not to overmatch
things like:

    test whatever && ls -a foo
    test whatever && foo -o outfile

I still do not understand why you have to insist on dashed operator
as the first thing given to "test", like this:

quoted

+	/\btest\s+-[a-z]\s+[^&|]+\s+-a\s+/ and err '"test A && test B" ...
+	/\btest\s+-[a-z]\s+[^&|]+\s+-o\s+/ and err '"test A || test B" ...

IOW, what over-matching would we get if we simplified the condition
like so?

    /\btest\s+[^&|]+\s+-a\s/
    /\btest\s+[^&|]+\s+-o\s/

The one in the patch would miss things like

	test "$a" = "$b" -o "$a" -lt "$b"
	test "$n" -a "$n" -lt 4

but the only thing that we care about is that a command that started
with "test " has "-a" or "-o" before we see "&" or "|", no?

Applying your suggestion results in these false positives:
	
	t4038-diff-combined.sh:135: error: "test A && test B" preferred to "test A -a B": git commit -m "test space change" -a &&
	t4038-diff-combined.sh:147: error: "test A && test B" preferred to "test A -a B": git commit -m "test other space changes" -a &&
	t6400-merge-df.sh:89: error: "test A || test B" preferred to "test A -o B": test 0 -eq $(git ls-files -o | wc -l)
	t6400-merge-df.sh:91: error: "test A || test B" preferred to "test A -o B": test 1 -eq $(git ls-files -o | wc -l)
	t6400-merge-df.sh:110: error: "test A || test B" preferred to "test A -o B": test 0 -eq $(git ls-files -o | wc -l)
	t6400-merge-df.sh:112: error: "test A || test B" preferred to "test A -o B": test 1 -eq $(git ls-files -o | wc -l)
	t6402-merge-rename.sh:639: error: "test A || test B" preferred to "test A -o B": test 0 -eq "$(git ls-files -o | wc -l)"
	t6402-merge-rename.sh:646: error: "test A || test B" preferred to "test A -o B": test 2 -eq "$(git ls-files -o | wc -l)"
	t6402-merge-rename.sh:686: error: "test A || test B" preferred to "test A -o B": test 0 -eq "$(git ls-files -o | wc -l)" &&
	t6402-merge-rename.sh:865: error: "test A || test B" preferred to "test A -o B": test 0 -eq $(git ls-files -o | wc -l) &&

With: 7dbe8c8003, (check-non-portable-shell.pl: `wc -l` may have
leading WS, 2017-12-21)
Unless the situation has been changed, since.
I think those tests with quoted "$(.. | wc -l)" has been deemed
unportable and should be replaced with test_line_count anyway?
Does "test -eq" strip spaces from integer strings?

And I think we're working on moving "git" command to its own commmand
instead of put it in the left of a pipe.

2 followed patch will clean them out

I think those patches are good in their own right, i.e. replacing things
with more incremental helpers and test_cmp-like functions.

But I believe the code you're changing is not non-portable. It was using
the output of "wc -l" with the "=" operator that wasn't portable.

These ones are all occurances that use "-eq".

And:

    test "0" -eq " 0"

etc., is true, which is why these pass on OSX and beyond.