Re: [PATCH v3] completion: add new _GIT_complete helper

[Felipe Contreras <felipe.contreras@xxxxxxxxx>]

On Sun, May 6, 2012 at 2:12 PM, SZEDER Gábor <szeder@xxxxxxxxxx> wrote:
> Hi,
>
>
> On Sun, May 06, 2012 at 01:14:25PM +0200, SZEDER Gábor wrote:
>> On Sat, May 05, 2012 at 05:23:20PM +0200, Felipe Contreras wrote:
>> > +__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
>> > +   __git_func "$@"
>> > +}
>> > +
>> > +_GIT_complete ()
>> > +{
>> > +   local name="${2-$1}"
>> > +   eval "$(typeset -f __git_func_wrap | sed -e "s/__git_func/_$name/")"
>>
>> Still don't like the subshell and sed here ...
>>
>> > +   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
>>
>> ... because it adds delay when the completion script is loaded.  But I
>> still don't have ideas how to avoid them.
>
> Ok, I think I got it.  How about this on top of Felipe's patch?
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index f300b87d..8c18db92 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2688,19 +2688,19 @@ __git_func_wrap ()
>        fi
>        local cur words cword prev
>        _get_comp_words_by_ref -n =: cur words cword prev
> -       __git_func "$@"
> +       $1
>  }
>
>  _GIT_complete ()
>  {
> -       local name="${2-$1}"
> -       eval "$(typeset -f __git_func_wrap | sed -e "s/__git_func/_$name/")"
> -       complete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \
> -               || complete -o default -o nospace -F _${name}_wrap $1
> +       local wrapper="__git_wrap_$1"
> +       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
>  }

This has unnecessary changes, and can be simplified this way:

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index f300b87..dd1ff33 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2688,13 +2688,13 @@ __git_func_wrap ()
        fi
        local cur words cword prev
        _get_comp_words_by_ref -n =: cur words cword prev
-       __git_func "$@"
+       _$1
 }

 _GIT_complete ()
 {
        local name="${2-$1}"
-       eval "$(typeset -f __git_func_wrap | sed -e "s/__git_func/_$name/")"
+       eval "_${name}_wrap () { __git_func_wrap $name ; }"
        complete -o bashdefault -o default -o nospace -F _${name}_wrap
$1 2>/dev/null \
                || complete -o default -o nospace -F _${name}_wrap $1
 }

Of course, it might make sense to use the $wrapper variable, but that
increases the diff, so I avoided it for reviewing purposes. And I
still see no point forcing users to specify the full name of the
function; they should not be bothered with such details.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html