Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.

From: Boyd Stephen Smith Jr. <bss@iguanasuicide.net>
Date: Jan 13, 2009, 03:56 UTC
Message-ID: <200901122157.03658.bss@iguanasuicide.net>
In-Reply-To: <7vy6xfew2n.fsf@gitster.siamese.dyndns.org>

On Monday 2009 January 12 21:14:40 Junio C Hamano wrote:

>Ted Pavlic <ted@tedpavlic.com> writes:
>> A vim modeline has also been added for consistency.
>
>Yuck.

While I dislike emacs and use vim pretty much exclusively, I don't really see the need for a vim modeline. On top of that, fdm=marker is a bit silly since there aren't any markers. ff=unix and ft=sh are redundant, as any vim should detect these properly, given the filename.

So, I'm slightly negative on the modeline hunk.

Show 10 quoted lines

>> @@ -111,7 +115,7 @@ __git_ps1 ()
>>  			fi
>>  		fi
>>
>> -		if [ -n "$1" ]; then
>> +		if [ $# -gt 0 ] && [ -n "$1" ]; then
>
>I found the previous round's [ -n "${1-}" ] much easier to read, if we were
> to do this.  If -n "${1-}", then "$1" is definitely set so nothing need to
> change in the then ... else part.

I found "${1-}" ugly, and this a bit better, but I'll defer to Junio.

Show 26 quoted lines

>> @@ -131,11 +136,22 @@ __gitcomp_1 ()
>>  	done
>>  }
>>
>> +# __gitcomp accepts 1, 2, 3, or 4 arguments
>> +# generates completion reply with compgen
>>  __gitcomp ()
>>  {
>> -	local cur="${COMP_WORDS[COMP_CWORD]}"
>> -	if [ $# -gt 2 ]; then
>> -		cur="$3"
>> +	local one two cur="${COMP_WORDS[COMP_CWORD]}" four
>> +	if [ $# -gt 0 ]; then
>> +		one="$1"
>> +		if [ $# -gt 1 ]; then
>> +			two="$2"
>> +			if [ $# -gt 2 ]; then
>> +				cur="$3"
>> +				if [ $# -gt 3 ]; then
>> +					four="$4"
>> +				fi
>> +			fi
>> +		fi
>>  	fi
>
>Yuck.

Definitely agreeing with Junio here. This is far less ascetic than the old patch. Truth be told, this whole thread would probably have been more productive without me chiming in. Sorry about that.

>If you are taking advantage of the fact that "local one" 
>will bind one to emptiness anyway, can't you do something like:
>
>	local one=${1-} two=${2-} cur=${3-} four=${4-}

Even better to use variable names that match the usage, if possible.

-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/