Re: [PATCH 10/10] git-submodule.sh: don't use the -a or -b option with the test command

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2014, 19:09 UTC
Message-ID: <xmqq7g5la6c6.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140515155821.GA27279@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 12 quoted lines

> Perhaps something like the following would work?
>
> 	tree-wide: convert test -a/-o to && and ||
>
> 	The interaction with unary operators and operator precedence
> 	for && and || are better known than -a and -o, and for that
> 	reason we prefer them.  Replace all existing instances in git
> 	of -a and -o to save readers from the burden of thinking
> 	about such things.
>
> 	Add a check-non-portable-shell.pl to avoid more instances of
> 	test -a and -o arising in the future.

Yeah, the title is certainly better than "-a or -b option" I see above ;-) and a single tree-wide fix may be OK while the tree is quiescent.

I however do think "better known" is much less of an issue than that "-a/-o" is more error prone e.g. 'test -n "$x" -a a = b' is buggy because it does not consider that $x could be "=".

Show 12 quoted lines

> [...]
>> -			test $status = D -o $status = T && echo "$sm_path" && continue
>> +			 ( test $status = D || test $status = T ) && echo "$sm_path" && continue
>
> There's no need for a subshell for this.  Better to use a block:
>
> 			{
> 				test "$status" = D ||
> 				test "$status" = T
> 			} &&
> 			echo "$sm_path" &&
> 			continue

Yes.