threads / patch / 29471

patchcompletion: cleanup __gitcomp*

Subject: [PATCH] completion: cleanup __gitcomp*

## tl;dr

3 messages between Jan 30, 2012 and Jan 30, 2012. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Felipe Contreras· Jan 30, 2012, 00:29 UTC · lore

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(-)

Show changes to contrib/completion/git-completion.bash +5 −19

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

Jonathan Nieder· Jan 30, 2012, 02:42 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: cleanup __gitcomp*

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

Felipe Contreras· Jan 30, 2012, 03:39 UTC · re: Jonathan Nieder · lore

Re: [PATCH] completion: cleanup __gitcomp*

On Mon, Jan 30, 2012 at 4:42 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 28 quoted lines

> 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