From: Junio C Hamano <gitster@pobox•com>
To: Jonathan Nieder <jrnieder@gmail•com>
Cc: Elia Pinto <gitter.spiros@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH 10/10] git-submodule.sh: don't use the -a or -b option with the test command
Date: Fri, 16 May 2014 12:09:29 -0700 [thread overview]
Message-ID: <xmqq7g5la6c6.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140515155821.GA27279@google.com> (Jonathan Nieder's message of "Thu, 15 May 2014 08:58:22 -0700")
Jonathan Nieder <jrnieder@gmail•com> writes:
> 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 "=".
> [...]
>> - 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.
prev parent reply other threads:[~2014-05-16 19:09 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-15 14:17 [PATCH 01/10] check_bindir: don't use the -a or -b option with the test command Elia Pinto
2014-05-15 14:17 ` [PATCH 02/10] contrib/examples/git-clone.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 03/10] contrib/examples/git-commit.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 04/10] contrib/examples/git-merge.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 05/10] contrib/examples/git-repack.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 06/10] contrib/examples/git-resolve.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 07/10] git-bisect.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 08/10] git-mergetool.sh: " Elia Pinto
2014-05-16 8:24 ` David Aguilar
2014-05-15 14:17 ` [PATCH 09/10] git-rebase--interactive.sh: " Elia Pinto
2014-05-15 14:17 ` [PATCH 10/10] git-submodule.sh: " Elia Pinto
2014-05-15 14:19 ` Elia Pinto
2014-05-15 15:58 ` Jonathan Nieder
2014-05-16 19:09 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq7g5la6c6.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitter.spiros@gmail$(echo .)com \
--cc=jrnieder@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox