Re: [PATCH v5] completion: add new __git_complete helper

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v5] completion: add new __git_complete helper

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:49

Felipe Contreras [off-list ref] writes:
quoted hunk
This simplifies the completions, and would make it easier to define
aliases in the future.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   70 +++++++++++++++-----------------
 t/t9902-completion.sh                  |    2 +-
 2 files changed, 34 insertions(+), 38 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f56ec7..d60bb8a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2603,21 +2603,6 @@ _git ()
 {
 	local i c=1 command __git_dir
 
-	if [[ -n ${ZSH_VERSION-} ]]; then
-		emulate -L bash
-		setopt KSH_TYPESET
-
-		# workaround zsh's bug that leaves 'words' as a special
-		# variable in versions < 4.3.12
-		typeset -h words
-
-		# workaround zsh's bug that quotes spaces in the COMPREPLY
-		# array if IFS doesn't contain spaces.
-		typeset -h IFS
-	fi
-
-	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -2667,22 +2652,6 @@ _git ()
 
 _gitk ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
-		emulate -L bash
-		setopt KSH_TYPESET
-
-		# workaround zsh's bug that leaves 'words' as a special
-		# variable in versions < 4.3.12
-		typeset -h words
-
-		# workaround zsh's bug that quotes spaces in the COMPREPLY
-		# array if IFS doesn't contain spaces.
-		typeset -h IFS
-	fi
-
-	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
-
@@ -2703,16 +2672,43 @@ _gitk ()
 	__git_complete_revlist
 }
 
-complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
-	|| complete -o default -o nospace -F _git git
-complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
-	|| complete -o default -o nospace -F _gitk gitk
Nice code reduction by moving the duplicated code into the new helpers ;-)
+__git_func_wrap ()
+{
+	if [[ -n ${ZSH_VERSION-} ]]; then
+		emulate -L bash
+		setopt KSH_TYPESET
+
+		# workaround zsh's bug that leaves 'words' as a special
+		# variable in versions < 4.3.12
+		typeset -h words
+
+		# workaround zsh's bug that quotes spaces in the COMPREPLY
+		# array if IFS doesn't contain spaces.
+		typeset -h IFS
+	fi
+	local cur words cword prev
+	_get_comp_words_by_ref -n =: cur words cword prev
+	$1
+}
+
+# Setup completion for certain functions defined above by setting common
+# variables and workarounds.
+# This is NOT a public function; use at your own risk.
+__git_complete ()
+{
+	local wrapper="__git_wrap${2}"
+	eval "$wrapper () { __git_func_wrap $2 ; }"
+	complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
+		|| complete -o default -o nospace -F $wrapper $1
+}
+
+__git_complete git _git
+__git_complete gitk _gitk
It makes my stomach queasy whenever I see $var not in double quotes that
forces me to guess (and trace to verify if the codepath is what I really
care about) if any value with $IFS in it could be used there, so even when
they are known to be safe, I'd prefer to see either explicit quotes or
comment that says what are expected in $1 and $2.

The quoting of all the added lines looks correct, though.
quoted hunk
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
-	|| complete -o default -o nospace -F _git git.exe
+__git_complete git.exe _git
 fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5bda6b6..0f09fd6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -63,7 +63,7 @@ run_completion ()
 	local _cword
 	_words=( $1 )
 	(( _cword = ${#_words[@]} - 1 ))
-	_git && print_comp
+	__git_wrap_git && print_comp
 }
 
 test_completion ()

Re: [PATCH v5] completion: add new __git_complete helper

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:49

On Mon, May 14, 2012 at 7:43 PM, Junio C Hamano [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
This simplifies the completions, and would make it easier to define
aliases in the future.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   70 +++++++++++++++-----------------
 t/t9902-completion.sh                  |    2 +-
 2 files changed, 34 insertions(+), 38 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f56ec7..d60bb8a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2603,21 +2603,6 @@ _git ()
 {
      local i c=1 command __git_dir

-     if [[ -n ${ZSH_VERSION-} ]]; then
-             emulate -L bash
-             setopt KSH_TYPESET
-
-             # workaround zsh's bug that leaves 'words' as a special
-             # variable in versions < 4.3.12
-             typeset -h words
-
-             # workaround zsh's bug that quotes spaces in the COMPREPLY
-             # array if IFS doesn't contain spaces.
-             typeset -h IFS
-     fi
-
-     local cur words cword prev
-     _get_comp_words_by_ref -n =: cur words cword prev
      while [ $c -lt $cword ]; do
              i="${words[c]}"
              case "$i" in
@@ -2667,22 +2652,6 @@ _git ()
 _gitk ()
 {
-     if [[ -n ${ZSH_VERSION-} ]]; then
-             emulate -L bash
-             setopt KSH_TYPESET
-
-             # workaround zsh's bug that leaves 'words' as a special
-             # variable in versions < 4.3.12
-             typeset -h words
-
-             # workaround zsh's bug that quotes spaces in the COMPREPLY
-             # array if IFS doesn't contain spaces.
-             typeset -h IFS
-     fi
-
-     local cur words cword prev
-     _get_comp_words_by_ref -n =: cur words cword prev
-
@@ -2703,16 +2672,43 @@ _gitk ()
      __git_complete_revlist
 }

-complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
-     || complete -o default -o nospace -F _git git
-complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
-     || complete -o default -o nospace -F _gitk gitk
Nice code reduction by moving the duplicated code into the new helpers ;-)
quoted
+__git_func_wrap ()
+{
+     if [[ -n ${ZSH_VERSION-} ]]; then
+             emulate -L bash
+             setopt KSH_TYPESET
+
+             # workaround zsh's bug that leaves 'words' as a special
+             # variable in versions < 4.3.12
+             typeset -h words
+
+             # workaround zsh's bug that quotes spaces in the COMPREPLY
+             # array if IFS doesn't contain spaces.
+             typeset -h IFS
+     fi
+     local cur words cword prev
+     _get_comp_words_by_ref -n =: cur words cword prev
+     $1
+}
+
+# Setup completion for certain functions defined above by setting common
+# variables and workarounds.
+# This is NOT a public function; use at your own risk.
+__git_complete ()
+{
+     local wrapper="__git_wrap${2}"
+     eval "$wrapper () { __git_func_wrap $2 ; }"
+     complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
+             || complete -o default -o nospace -F $wrapper $1
+}
+
+__git_complete git _git
+__git_complete gitk _gitk
It makes my stomach queasy whenever I see $var not in double quotes that
forces me to guess (and trace to verify if the codepath is what I really
care about) if any value with $IFS in it could be used there, so even when
they are known to be safe, I'd prefer to see either explicit quotes or
comment that says what are expected in $1 and $2.
Which ones? These?
quoted
+     $1
+     complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
+             || complete -o default -o nospace -F $wrapper $1
So you want:

  "$1"

  complete -o bashdefault -o default -o nospace -F "$wrapper" "$1" 2>/dev/null \
    || complete -o default -o nospace -F "$wrapper" "$1"

I wouldn't object to the arguments of complete, but the function call
seems totally unintuitive to me like that.

-- 
Felipe Contreras
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help