Re: [PATCH v2 1/4] global: convert trivial usages of `test <expr> -a/-o <expr>`

From: Jeff King <peff@peff.net>
Date: Nov 10, 2023, 21:44 UTC
Message-ID: <20231110214423.GC2758295@coredump.intra.peff.net>
In-Reply-To: <2967c8ebb460934eb4aaaaebe5941bff643d4a94.1699609940.git.ps@pks.im>

On Fri, Nov 10, 2023 at 11:01:15AM +0100, Patrick Steinhardt wrote:

Show 11 quoted lines

> diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
> index 669ebaf68be..9fbf90cee7c 100755
> --- a/t/valgrind/valgrind.sh
> +++ b/t/valgrind/valgrind.sh
> @@ -23,7 +23,7 @@ memcheck)
>  	VALGRIND_MAJOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*\([0-9]*\)')
>  	VALGRIND_MINOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*[0-9]*\.\([0-9]*\)')
>  	test 3 -gt "$VALGRIND_MAJOR" ||
> -	test 3 -eq "$VALGRIND_MAJOR" -a 4 -gt "$VALGRIND_MINOR" ||
> +	( test 3 -eq "$VALGRIND_MAJOR" && test 4 -gt "$VALGRIND_MINOR" ) ||
>  	TOOL_OPTIONS="$TOOL_OPTIONS --track-origins=yes"

I was surprised to see this one as a subshell after you adjusted the other. It probably isn't that big a deal either way, though (as a style thing I generally try to use braces unless I am relying on the separate environment provided by the subshell, but it's certainly not wrong in this case).

-Peff