[pacman-dev] [PATCH 2/2] Changing [ to [[ and (( - Part Two - Amended

[Allan McRae]

I have looked at the two patches and the good news is that I can spot 
nothing wrong!  I just have a few comments about style that I would like 
to discuss.


I would like to get some more consistency in quoting. e.g. here are some 
varied tests:

-	if [ "$ret" != '?' ]; then
+	if [[ $ret != '?' ]]; then

+		if [[ $ret != "?" ]]; then
+			if [[ $ret = y ]]; then

-		if [ "$cmd" = "bsdtar" ]; then
+		if [[ $cmd = bsdtar ]]; then

-	if [ "$(check_option makeflags)" = "n" ]; then
+	if [[ $(check_option makeflags) = "n" ]]; then

-	if [ "$arch" != 'any' ]; then
+	if [[ $arch != 'any' ]]; then

-		if [ ${1:0:2} = '--' ]; then
+		if [[ ${1:0:2} = '--' ]]; then

-			if [ -d ./src/$_hgrepo ] ; then
+			if [[ -d ./src/$_hgrepo ]] ; then

-if [ -r ~/.makepkg.conf ]; then
+if [[ -r ~/.makepkg.conf ]]; then

My suggestion is that any thing with text (i.e. not a pure variable) is 
quoted.  I know this is excessive in some cases (e.g. the last case) but 
the only exception I would be happy with is tests that are pure paths 
with only added "/" (e.g. $startdir/$file).  Even then, maybe quotes 
would be nicer...  I am happy to be debated on this.



There should be quotes kept in the gettext calls in this test:
-		if [ "$answer" = "$(gettext "YES")" -o "$answer" = "$(gettext "Y")" 
]; then
+		if [[ $answer = $(gettext YES) || $answer = $(gettext Y) ]]; then



Why is EUID tested against 0 explicitly when all other tests for zero 
just use ! EUID? e.g.

-	if [ $EUID -eq 0 -a "$ASROOT" -eq 0 ]; then
+	if (( EUID == 0 && ! ASROOT )); then

In fact, I quite like that things are explicitly tested in this case.

I wonder if the tests of return values should explicitly test for "== 0" 
or "!= 0". e.g. these test have become less clear to me when I read the 
code.

-	if [ $ret -gt 0 ]; then
+	if (( ret )); then

-	elif [ $ret -ne 0 ]; then
+	elif (( ret )); then

Note that these explicitly test for "== 0" or "> 0" and I think that is 
much clearer:

-	[ $# -gt 0 ] || return
+	(( $# > 0 )) || return

-	[ $# -eq 0 ] && return $R_DEPS_SATISFIED
+	(( $# == 0 )) && return $R_DEPS_SATISFIED



Allan