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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 13, 2009, 03:14 UTC
Message-ID: <7vy6xfew2n.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <496C0003.7040909@tedpavlic.com>

Ted Pavlic <ted@tedpavlic.com> writes:

> A vim modeline has also been added for consistency.

Yuck.

Show 5 quoted lines

> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> ---
>  contrib/completion/git-completion.bash |   70
> +++++++++++++++++++++++++-------
>  1 files changed, 55 insertions(+), 15 deletions(-)

Double yuck.

Show 24 quoted lines

> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 7b074d7..619e886 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -50,9 +50,11 @@ case "$COMP_WORDBREAKS" in
>  *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
>  esac
>
> +# __gitdir accepts 0 or 1 arguments (i.e., location)
> +# returns location of .git repo
>  __gitdir ()
>  {
> -	if [ -z "$1" ]; then
> +	if [ $# -eq 0 ] || [ -z "$1" ]; then
>  		if [ -n "$__git_dir" ]; then
>  			echo "$__git_dir"
>  		elif [ -d .git ]; then
> @@ -67,6 +69,8 @@ __gitdir ()
>  	fi
>  }
>
> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> +# returns text to add to bash PS1 prompt (includes branch name)

Good.  Would be better if you described what $1 and $2 mean.

Show 6 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.

> +# __gitcomp_1 requires 2 arguments

... and $1 and $2 mean?

Show 27 quoted lines

>  __gitcomp_1 ()
>  {
>  	local c IFS=' '$'\t'$'\n'
> @@ -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. 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-}

to avoid this ugliness?