Re: [PATCH 10/12] completion: add new git_complete helper

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Apr 11, 2012, 22:50 UTC
Message-ID: <20120411225046.GU2289@goldbirke>
In-Reply-To: <1333854479-23260-11-git-send-email-felipe.contreras@gmail.com>

Hi,

On Sun, Apr 08, 2012 at 06:07:57AM +0300, Felipe Contreras wrote:

Show 56 quoted lines

> This simplifies the completions, and makes it easier to define aliases:
> 
>  git_complete gf git_fetch
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash |   69 +++++++++++++++-----------------
>  t/t9902-completion.sh                  |    2 +-
>  2 files changed, 33 insertions(+), 38 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 60ea224..6cf1d98 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2616,21 +2616,6 @@ _git ()
>  {
>  	local i c=1 cmd cmd_pos __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
> @@ -2683,22 +2668,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
> -

Eliminating code duplication, great!

Show 11 quoted lines

>  	__git_has_doubledash && return
>  
>  	local g="$(__gitdir)"
> @@ -2719,16 +2688,42 @@ _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

More greatness.

> +foo_wrap ()

This git-completion-specific internal helper function will pollute the user's environment. It should get a __git_ prefix.

Show 20 quoted lines

> +{
> +	local cmd=foo_cmd cmd_pos=1
> +	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
> +	foo "$@"
> +}
> +
> +git_complete ()

Maybe this should also get a prefix, but I'm not sure, because this is, strictly speaking, not a bash completion function.

Show 10 quoted lines

> +{
> +	local name="${2-$1}"
> +	local cmd="${name#git_}"
> +	eval "$(typeset -f foo_wrap | sed -e "s/foo_cmd/$cmd/" -e "s/foo/_$name/")"
> +	complete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \
> +		|| complete -o default -o nospace -F _${name}_wrap $1
> +}
> +
> +git_complete git
> +git_complete gitk

Clever ;)

But it needs a subshell and a sed invocation, so Windows people might not be all too happy about this. Perhaps it's not a big deal in the default case, because it will be done only twice at startup time. However, if someone wants several aliases on Windows, it can cause noticeable delay. It's a startup speed vs. convenience trade-off. Anyway, I don't have any ideas how those fork()s and exec() could be avoided.

Best, Gábor