Re: [PATCH v2 3/4] completion: cleanup __gitcomp*

Jonathan Nieder [off-list ref] writes:

I imagine it would have been enough to say something along the lines of
"The __gitcomp and __gitcomp_nl functions are unnecessarily verbose.
__gitcomp_nl sets IFS to " \t\n" unnecessarily before setting it to "\n"
by mistake.  Both functions use 'if' statements to read parameters
with defaults, where the ${parameter:-default} idiom would be just as
clear.  By fixing these, we can make each function almost a one-liner."

By the way, the subject ("clean up __gitcomp*") tells me almost as
little as something like "fix __gitcomp*".  A person reading the
shortlog would like to know _how_ you are fixing it, or what the
impact of the change will be --- e.g., something like "simplify
__gitcomp and __gitcomp_nl" would be clearer.

I love both of the above two paragraphs.  Thanks.

[...]

quoted

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

[...]

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}"))

This loses the nice name $suffix for the -S argument.  Not a problem,
just noticing.

The patch looks good, including the localness that is kept for IFS.