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

["SZEDER Gábor" <szeder@ira.uka.de>]

Hi,


On Mon, May 07, 2012 at 02:20:54AM +0200, Felipe Contreras wrote:
> In fact, it could be even simpler:
> 
> _GIT_complete gf fetch

That would be even better; just need to take care of 'cherry-pick' and
the like.

> >> > 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'.

> It does work... on my branch.

I though the patch was supposed to be applied on git.git's master.  I
didn't know what else you had on your branch, obviously.

> If you have another example that works, feel free to suggest it

I played around with 'gb' as 'git branch' and 'gc' as 'git checkout',
they seemed to work properly, and they both use $words directly or
indirectly.  I suspect most of them Just Works, except those using
__git_complete_remote_or_refspec(), and _git_bundle() as you mention
below, and perhaps a few surprises we hadn't spotted yet.

> >> 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.
> 
> ${words[1]} is part of $words.

The problem is not with $words per se, but with the '1'.

> And BTW, git bundle also fails similarly.

Indeed; using __git_find_on_cmdline() and checking which subcommand it
found, if any, would allow us to get rid of all numbers from that
function.

> Also BTW, git fetch is already broken anyway:
> 
>  git --no-pager fetch <TAB>
> 
> So don't blame my patch :)

Yeah, I know, but that's broken since "forever".  However, I think
that would be a separate, though related topic, because it's not
required to make completion for alias work.

> >> > 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.

> That's not enough to make 'git fetch' work, try it.

As already mentioned it earlier in this thread, the iteration over
$words in __git_complete_remote_or_refspec() have to be fixed, too.

> >> >> +     __git_func "$@"

> Sure, but it was just 4 more characters that didn't hurt anybody. Your
> version makes passing those arguments more difficult, so I see no need
> to try to implement that.

OK.  I doesn't hurt, but I think it's more important to avoid
fork()+exec() overheads than to pass arguments whose values are
readily available in other variables anyway.


Gábor