Re: [PATCH v3] completion: add new _GIT_complete helper
- From
SZEDER Gábor <szeder@ira.uka.de>
- Date
- May 6, 2012, 23:32 UTC
- Message-ID
- <20120506233235.GN2164@goldbirke>
- In-Reply-To
- <CAMP44s3xwgsfjZA6r+bydu-5r1nEO5cGc=wcDSr+WdwunTtpwg@mail.gmail.com>
Hi,
On Sun, May 06, 2012 at 10:37:06PM +0200, Felipe Contreras wrote:
Show 20 quoted lines
> On Sun, May 6, 2012 at 1:14 PM, SZEDER Gábor <szeder@ira.uka.de> 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
Show 6 quoted lines
> > 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.
Show 10 quoted lines
> > 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.
Show 7 quoted lines
> >> + __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