[pacman-dev] [PATCH 1/2] Changing [ to [[ and ((

[Cedric Staniewski]

Isaac Good wrote:
> On Thu, Nov 12, 2009 at 10:37:31PM +0000, Cedric Staniewski wrote:
>> Isaac Good wrote:
>>> On Thu, Nov 12, 2009 at 07:16:41PM +0000, Cedric Staniewski wrote:
>>>> I hope your patch do not get lost in this thread.
>>> Erm. Yeah, I hope so too. How does it get threaded?
>>>
>> I guess you hit the reply button instead of writing a new mail to
>> pacman-dev. At least, there is the In-Reply-To header field in your mail
>> header.
>>
>>>>> @@ -345,26 +345,26 @@ handle_deps() {
>>>>>  	local R_DEPS_SATISFIED=0
>>>>>  	local R_DEPS_MISSING=1
>>>>>  
>>>>> -	[ $# -eq 0 ] && return $R_DEPS_SATISFIED
>>>>> +	(( $# == 0 )) && return $R_DEPS_SATISFIED
>>>>>
>>>> Is there a reason why you do not use (( ! $# )) here?
>>> Some variables are used as a boolean flag in which case testing if ((VAR)) and if (( ! VAR )) reads well. The $# (and EUID) are used as integer values so it struck me as more understandable or readable in this form.
>>>
>> I am fine with that, but would it not makes sense to use (( $# > 0 ))
>> instead of just (( $# )) then?
> 
> I am not sure I understand what you are saying.
> In general there are no negative values. With boolean flags either it is set or not set. So you get : if (( FLAG )) and if (( ! FLAG ))
> With $# (and EUID) the code is set up to test for zero, not non-zero. (( $# > 0 )) and (( $# )) are the reverse of what the code checks for.
> All of these do the same thing:
> $ (( $# == 0 )) && return $R_DEPS_SATISFIED
> $ (( $# > 0 )) || return $R_DEPS_SATISFIED
> $ (( ! $# ))  && return $R_DEPS_SATISFIED
> $ (( $# )) || return $R_DEPS_SATISFIED
> 
> I left the test as && because there was no compelling reason to change it. That aside, I am in favor of either of the first two for $# as opposed to the latter two for boolean flags.
> 

Sorry, I meant another line from your patch where you use (( $# )).

>  }
>  
>  check_deps() {
> -	[ $# -gt 0 ] || return
> +	(( $# )) || return
>  
>  	pmout=$(pacman $PACMAN_OPTS -T "$@")
>  	ret=$?
> -	if [ $ret -eq 127 ]; then #unresolved deps
> +	if (( ret == 127 )); then #unresolved deps
>  		echo "$pmout"
> -	elif [ $ret -ne 0 ]; then
> +	elif (( ret )); then
>  		error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout"
>  		exit 1
>  	fi

ret is another non-boolean variable by the way. I think some kind of
code style guide is a good idea. :)