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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: May 7, 2012, 09:22 UTC
Message-ID: <20120507092213.GO2164@goldbirke>
In-Reply-To: <CAMP44s3q-X7W5qeu6LZzu13C6SfhEZy9i4AfJ9Sszcou52MsGw@mail.gmail.com>

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.

Show 8 quoted lines

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

Show 5 quoted lines

> 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