[PATCH] completion: cleanup __gitcomp*

[PATCH] completion: cleanup __gitcomp*

[Felipe Contreras]

I don't know why there's so much code; these functions don't seem to be
doing much:

 * ${1-} is the same as $1
 * no need to check $#, ${3:-$cur} is much easier
 * __gitcomp_nl doesn't seem to using the initial IFS

This makes the code much simpler.

Eventually it would be nice to wrap everything that touches compgen and
COMPREPLY in one function for the zsh wrapper.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   24 +++++-------------------
 1 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..accfce5 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -495,19 +495,15 @@ 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=()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "${2-}" \
-			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
+		COMPREPLY=($(compgen -P "$2" \
+			-W "$(__gitcomp_1 "$1" "$4")" \
 			-- "$cur_"))
 		;;
 	esac
@@ -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}"))
 }
 
 __git_heads ()
-- 
1.7.8.3

Re: [PATCH] completion: cleanup __gitcomp*

[Jonathan Nieder]

Felipe Contreras wrote:

>  * ${1-} is the same as $1

| $ git log -S'${1-}' contrib/completion/git-completion.bash
| [...]
| commit 25a31f81
| Author: Ted Pavlic <ted@tedpavlic.com>
| Date:   Thu Jan 15 11:02:21 2009 -0500
|
|     bash-completion: Support running when set -u is enabled
|
|     Under "set -u" semantics, it is an error to access undefined variables.
|     Some user environments may enable this setting in the interactive shell.
|
|     In any context where the completion functions access an undefined
|     variable, accessing a default empty string (aka "${1-}" instead of "$1")
|     is a reasonable way to code the function, as it silences the undefined
|     variable error while still supplying an empty string.
|
|     In this patch, functions that should always take an argument still use
|     $1. Functions that have optional arguments use ${1-}.
|
|     Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
|     Acked-by: Shawn O. Pearce <spearce@spearce.org>
|     Signed-off-by: Junio C Hamano <gitster@pobox.com>

Hope that helps,
Jonathan

Re: [PATCH] completion: cleanup __gitcomp*

[Felipe Contreras]

On Mon, Jan 30, 2012 at 4:42 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>>  * ${1-} is the same as $1
>
> | $ git log -S'${1-}' contrib/completion/git-completion.bash
> | [...]
> | commit 25a31f81
> | Author: Ted Pavlic <ted@tedpavlic.com>
> | Date:   Thu Jan 15 11:02:21 2009 -0500
> |
> |     bash-completion: Support running when set -u is enabled
> |
> |     Under "set -u" semantics, it is an error to access undefined variables.
> |     Some user environments may enable this setting in the interactive shell.
> |
> |     In any context where the completion functions access an undefined
> |     variable, accessing a default empty string (aka "${1-}" instead of "$1")
> |     is a reasonable way to code the function, as it silences the undefined
> |     variable error while still supplying an empty string.
> |
> |     In this patch, functions that should always take an argument still use
> |     $1. Functions that have optional arguments use ${1-}.
> |
> |     Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> |     Acked-by: Shawn O. Pearce <spearce@spearce.org>
> |     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> Hope that helps,

I see. I'll revert that then.

-- 
Felipe Contreras