Re: [PATCH] bash-completion: Support running with set -u in bash 4.0

[Jonathan Nieder <jrnieder@gmail.com>]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> @@ -2181,7 +2181,7 @@ _git ()
>>  		c=$((++c))
>>  	done
>>  
>> -	if [ -z "$command" ]; then
>> +	if [ -z "${command-}" ]; then
>>  		case "${COMP_WORDS[COMP_CWORD]}" in
>>  		--*)   __gitcomp "
>>  			--paginate
>
> Why not this patch, instead of the above hunk?

My mistake.  The original patch was as you suggest, then I changed it
to the above to make it follow the style of surrounding code.

If one makes it a goal to lose the ${foo-} lines:

 - In __gitdir, [ -z "${1-}" ] should be [ $# -eq 0 ]
 - In __gitdir callers, "${1-}" should be replaced by "$@"
 - In _git, __git_dir should be initialized to ""
 - In _gitk and __git_ps1, __git_dir should be made local and
   initialized (independent bugfix)
 - In __git_ps1, the code examining $GIT_PS1_DESCRIBE_STYLE
   should be protected by an if [ -n "${GIT_PS1_DESCRIBE_STYLE:+set}" ]
 - The checks for GIT_PS1_SHOWDIRTYSTATE and GIT_PS1_SHOWUNTRACKEDFILES
   would also be adjusted
 - __gitcomp should get some local variables set to "" and then
   conditionally set to its arguments
 - "${1-}" in __git_heads and __git_tags should be "$dir"

These look like good things to do anyway.  Assuming your change
has already been made, the follow-up patch would look something
like this.

-- %< --
Subject: bash-completion: Avoid using uninitialized values

The "${var-}" idiom is obscure and makes it easy to hide uses of
values not initialized by the completion script that could have leaked
from the user’s environment.

Untested.  The only intentional change in functionality from this
patch is that __git_dir has been made local to _gitk() and __git_ps1.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 contrib/completion/git-completion.bash |   52 +++++++++++++++++++++-----------
 1 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2682f52..e291d8d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -63,8 +63,8 @@ esac
 # returns location of .git repo
 __gitdir ()
 {
-	if [ -z "${1-}" ]; then
-		if [ -n "${__git_dir-}" ]; then
+	if [ $# -eq 0 ]; then
+		if [ -n "$__git_dir" ]; then
 			echo "$__git_dir"
 		elif [ -d .git ]; then
 			echo .git
@@ -82,6 +82,7 @@ __gitdir ()
 # returns text to add to bash PS1 prompt (includes branch name)
 __git_ps1 ()
 {
+	local __git_dir=""
 	local g="$(__gitdir)"
 	if [ -n "$g" ]; then
 		local r=""
@@ -108,18 +109,20 @@ __git_ps1 ()
 			fi
 
 			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-
-				b="$(
-				case "${GIT_PS1_DESCRIBE_STYLE-}" in
+				if [ -z "${GIT_PS1_DESCRIBE_STYLE:+set}" ] ||
+					[ "$GIT_PS1_DESCRIBE_STYLE" = default ]; then
+					b=$(git describe --exact-match HEAD 2>/dev/null)
+				else b=$(case "$GIT_PS1_DESCRIBE_STYLE" in
 				(contains)
 					git describe --contains HEAD ;;
 				(branch)
 					git describe --contains --all HEAD ;;
 				(describe)
 					git describe HEAD ;;
-				(* | default)
+				(*)
 					git describe --exact-match HEAD ;;
-				esac 2>/dev/null)" ||
+				esac 2>/dev/null)
+				fi ||
 
 				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
 				b="unknown"
@@ -140,7 +143,7 @@ __git_ps1 ()
 				b="GIT_DIR!"
 			fi
 		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
-			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
+			if [ -n "${GIT_PS1_SHOWDIRTYSTATE:+set}" ]; then
 				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
 					git diff --no-ext-diff --quiet --exit-code || w="*"
 					if git rev-parse --quiet --verify HEAD >/dev/null; then
@@ -150,11 +153,11 @@ __git_ps1 ()
 					fi
 				fi
 			fi
-			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+			if [ -n "${GIT_PS1_SHOWSTASHSTATE:+set}" ]; then
 			        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
 			fi
 
-			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
+			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES:+set}" ]; then
 			   if [ -n "$(git ls-files --others --exclude-standard)" ]; then
 			      u="%"
 			   fi
@@ -183,18 +186,30 @@ __gitcomp_1 ()
 # generates completion reply with compgen
 __gitcomp ()
 {
+	local replies=""
+	local pfx=""
 	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local sfx=""
+	if [ $# -gt 3 ]; then
+		sfx=$4
+	fi
 	if [ $# -gt 2 ]; then
 		cur="$3"
 	fi
+	if [ $# -gt 1 ]; then
+		pfx=$2
+	fi
+	if [ $# -gt 0 ]; then
+		replies=$1
+	fi
 	case "$cur" in
 	--*=)
 		COMPREPLY=()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "${2-}" \
-			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
+		COMPREPLY=($(compgen -P "$pfx" \
+			-W "$(__gitcomp_1 "$replies" "$sfx")" \
 			-- "$cur"))
 		;;
 	esac
@@ -203,13 +218,13 @@ __gitcomp ()
 # __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local cmd i is_hash=y dir="$(__gitdir "$@")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
+	for i in $(git ls-remote "$dir" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -222,13 +237,13 @@ __git_heads ()
 # __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
 __git_tags ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local cmd i is_hash=y dir="$(__gitdir "$@")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
+	for i in $(git ls-remote "$dir" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -241,7 +256,7 @@ __git_tags ()
 # __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
 __git_refs ()
 {
-	local i is_hash=y dir="$(__gitdir "${1-}")"
+	local i is_hash=y dir="$(__gitdir "$@")"
 	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
@@ -2167,7 +2182,7 @@ _git_tag ()
 
 _git ()
 {
-	local i c=1 command="" __git_dir
+	local i c=1 command="" __git_dir=""
 
 	while [ $c -lt $COMP_CWORD ]; do
 		i="${COMP_WORDS[c]}"
@@ -2265,6 +2280,7 @@ _gitk ()
 {
 	__git_has_doubledash && return
 
+	local __git_dir=""
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	local g="$(__gitdir)"
 	local merge=""
-- 
1.7.0