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

[Isaac Good]

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.

> >>> @@ -512,7 +512,7 @@ generate_checksums() {
> >>>  
> >>>  		local i=0;
> >>>  		local indent=''
> >>> -		while [ $i -lt $((${#integ}+6)) ]; do
> >>> +		while [[ $i -lt $((${#integ}+6)) ]]; do
> >>>  			indent="$indent "
> >>>  			i=$(($i+1))
> >>>  		done
> >> What about "while (( $i < $((${#integ}+6)) )); do"?
> > 
> > I was planning a separate patch switching to a for loop.
> > for (( i = 0; i < ${#integ} + 6; i++ ))
> >
> 
> In this case, I would either get rid of -lt by converting to a for loop
>  directly or not touch that line at all in this patch.

amended/reversed

> > Should I implement these changes and resend this patch?
> > /me goes looking for git help 
> > 
> 
> If you like, but you could also wait for at least Allan's comment. You
> can use "git commit --amend" to alter your existing commit in git by the
> way.
 
Thanks. I'm learning a lot of git today ;)

 - Isaac