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

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> This simplifies the completions, and would make it easier to define
> aliases in the future.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  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.

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