diff mbox series

[OpenWrt-Devel,2/5] scripts/env: replace -a and -o with &&/||

Message ID 20200101020146.21043-2-rosenp@gmail.com
State Accepted
Headers show
Series None | expand

Commit Message

Rosen Penev Jan. 1, 2020, 2:01 a.m. UTC 
The former are not well defined.

Found with shellcheck.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 scripts/env | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Rosen Penev Jan. 1, 2020, 11:09 p.m. UTC | #1 
On Wed, Jan 1, 2020 at 5:47 AM Paul Oranje <por@oranjevos.nl> wrote:
>
> Happy new year !
>
> A minor question/nitpick, see below.
> Regards
>
> > Op 1 jan. 2020, om 03:01 heeft Rosen Penev <rosenp@gmail.com> het volgende geschreven:
> >
> > The former are not well defined.
> >
> > Found with shellcheck.
> [skip]
>
> > -                     [ -d "$BASEDIR/files" -a \! -L "$BASEDIR/files" ] && {
> > +                     if [ -d "$BASEDIR/files" ] && [ \! -L "$BASEDIR/files" ]; then
> >                               mkdir -p "$ENVDIR/files"
> >                               shopt -s dotglob
> >                               mv "$BASEDIR/files/"* "$ENVDIR/files/" 2>/dev/null
> >                               shopt -u dotglob
> >                               rmdir "$BASEDIR/files"
> > -                     }
> > +                     fi
> Why has " ... && { ... }" syntax been replaced with "if ... then ... fi" ?
> Both forms are used in the script anyway.
It's clearer. One issue with replacing -a with && is the case of [ 1
-a 2 ] ||. [ 1 ] && [ 2 ] || has the wrong behaviour. It's not the
case here but generally speaking it's more correct to replace with an
if statement.

Please also Reply to All next time.
>
> [skip]
diff mbox series

Patch

diff --git a/scripts/env b/scripts/env
index fd49e1c817..563f39c1cf 100755
--- a/scripts/env
+++ b/scripts/env
@@ -73,7 +73,7 @@  env_init() {
 }
 
 env_sync_data() {
-	[ \! -L "$BASEDIR/.config" -a -f "$BASEDIR/.config" ] && mv "$BASEDIR/.config" "$ENVDIR"
+	[ \! -L "$BASEDIR/.config" ] && [ -f "$BASEDIR/.config" ] && mv "$BASEDIR/.config" "$ENVDIR"
 	git add .
 	git add -u
 }
@@ -185,7 +185,7 @@  env_new() {
 	env_init 1
 	
 	branch="$(git branch | grep '^\* ' | awk '{print $2}')"
-	if [ -n "$branch" -a "$branch" != "master" ]; then
+	if [ -n "$branch" ] && [ "$branch" != "master" ]; then
 		env_ask_sync
 		if ask_bool 0 "Do you want to clone the current environment?"; then
 			from="$branch"
@@ -193,15 +193,15 @@  env_new() {
 		rm -f "$BASEDIR/.config" "$BASEDIR/files"
 	fi
 	git checkout -b "$1" "$from"
-	if [ -f "$BASEDIR/.config" -o -d "$BASEDIR/files" ]; then
+	if [ -f "$BASEDIR/.config" ] || [ -d "$BASEDIR/files" ]; then
 		if ask_bool 1 "Do you want to start your configuration repository with the current configuration?"; then
-			[ -d "$BASEDIR/files" -a \! -L "$BASEDIR/files" ] && {
+			if [ -d "$BASEDIR/files" ] && [ \! -L "$BASEDIR/files" ]; then
 				mkdir -p "$ENVDIR/files"
 				shopt -s dotglob
 				mv "$BASEDIR/files/"* "$ENVDIR/files/" 2>/dev/null
 				shopt -u dotglob
 				rmdir "$BASEDIR/files"
-			}
+			fi
 			env_sync
 		else
 			rm -rf "$BASEDIR/.config" "$BASEDIR/files"