Hi,
On Sun, May 06, 2012 at 10:37:06PM +0200, Felipe Contreras wrote:
> On Sun, May 6, 2012 at 1:14 PM, SZEDER Gábor <szeder@xxxxxxxxxx> wrote:
> > On Sat, May 05, 2012 at 05:23:20PM +0200, Felipe Contreras wrote:
> >> This simplifies the completions, and makes it easier to define aliases:
> >>
> >> _GIT_complete gf git_fetch
> >
> > So, 'gf' is an alias for 'git fetch', for which the user would like to
> > use the completion for 'git fetch', right? But that completion
> > function is caled _git_fetch(), so the underscore prefix is missing
> > here.
>
> No, it's not missing:
>
> 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
>
> See how we add '_' before $name?
Indeed, the '_' is added before $name no less than three times. How
could I have missed that?! ;) It would be better to do it once and be
done with it.
> There's not point in burdening the
> user with adding a prefix that will _always_ be there anyway.
I don't think it's that much of a burden. The function is called
_git_fetch, use that as second argument
> > Besides, this example won't work, because the completion for 'git
> > fetch' uses __git_complete_remote_or_refspec(), which in turn relies
> > on finding out the name of the git command from the word on the
> > command line, and it won't be able to do that from 'gf'.
>
> That's irrelevant, it's an example;
It's relevant; if you give an example, then at least that example
should work properly, don't you think?
> replace with another command that
> doesn't use 'words', and it would work.
That it doesn't work has nothing to do with $words. The problem is that
__git_complete_remote_or_refspec() expects to find the git command in
${words[1]}, but in case of an alias it can't.
> > I remember we discussed this in an earlier round, and you even
> > suggested a possible fix (passing the command name as argument to
> > __git_complete_remote_or_refspec()). I think that's the right thing
> > to do here.
>
> Yeah, but I suggested that in order to avoid the eval and the typeset
> that I require for future future patches, but it turns out it's still
> needed anyway, so my suggestion is to have a 'cmd' variable that
> stores the command; __git_func_wrap would take the responsibility of
> doing that.
Well, now I suggest to do that to fix
__git_complete_remote_or_refspec(), because that seems to be the
easiest, cleanest, and fastest solution.
> >> + __git_func "$@"
> >
> > What is this "$@" for and why? None of the _git_<cmd>() functions
> > take any arguments, nor does _git() and _gitk(), and AFAICT Bash won't
> > pass any either.
>
> bash's complete passes 3 arguments.
Oh, indeed; the first argument is the command name, the second is the
current word, and the third is the previous word. All these are
available in our completion functions as ${words[0]}, $cur, and $prev,
respectively.
> They might not be used, but it
> doesn't hurt to pass them either.
They _are_ not used, so passing them has no benefit either. I would
rather stick to using $cur and $prev than $2 and $3.
Best,
Gábor
--
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