Re: [PATCH v4 4/4] completion: simplify __gitcomp*

Felipe Contreras [off-list ref] writes:

I think this rewrite is wrong, even though it may not make a difference to
the current callers (I didn't check).  Drop the colon from ${3:-...}.

So is this.

Fixing the above two gives me what I've already sent in $gmane/189683,
so...

Hi,


On Fri, Feb 03, 2012 at 12:23:12PM -0800, Junio C Hamano wrote:

Felipe Contreras [off-list ref] writes:

quoted

@@ -495,11 +495,7 @@ fi
 # 4: A suffix to be appended to each possible completion word (optional).
 __gitcomp ()
 {
-	local cur_="$cur"
-
-	if [ $# -gt 2 ]; then
-		cur_="$3"
-	fi
+	local cur_="${3:-$cur}"
 	case "$cur_" in
 	--*=)
 		COMPREPLY=()

I think this rewrite is wrong, even though it may not make a difference to
the current callers (I didn't check).  Drop the colon from ${3:-...}.

quoted

@@ -524,18 +520,8 @@ __gitcomp ()
 #    appended.
 __gitcomp_nl ()
 {
-	local s=$'\n' IFS=' '$'\t'$'\n'
-	local cur_="$cur" suffix=" "
-
-	if [ $# -gt 2 ]; then
-		cur_="$3"
-		if [ $# -gt 3 ]; then
-			suffix="$4"
-		fi
-	fi
-
-	IFS=$s
-	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
+	local IFS=$'\n'
+	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3:-$cur}"))

So is this.

Fixing the above two gives me what I've already sent in $gmane/189683,
so...

Good point, I missed this when pointed out the similar issue with $4
earlier.

And it does make a difference, it breaks the completion of a single
word in multiple steps, e.g. git log --pretty=<TAB> master..<TAB>.  In
such cases we pass "${cur##--pretty=}" and "${cur_#*..}" as third
argument to __gitcomp() and __gitcomp_nl(), which can be empty strings
when the user hits TAB right after the '=' and '..'.  Replacing that
empty string with $cur is bad, because none of the possible completion
words (i.e. $1) will match it, and bash will fall back to filename
completion.

Without the colon, i.e. using "${3-$cur}", it works as expected.


Best,
Gábor