git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Mar 6, 2010, 20:17 UTC
Message-ID
<20100306201753.GA4704@progeny.tock>
In-Reply-To
<7vtystfdu5.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
Show 11 quoted lines
>> @@ -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
Previous: Junio C Hamano
Message 3 of 3 in “bash-completion: Support running with set -u in bash 4.0”
  1. bash-completion: Support running with set -u in bash 4.0Jonathan Nieder, Mar 6, 2010
  2. Junio C HamanoMar 6, 2010
  3. Jonathan NiederMar 6, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.