Re: [PATCHv2 3/3] completion: match ctags symbol names in grep patterns

From: Jeff King <peff@peff.net>
Date: Nov 1, 2011, 15:21 UTC
Message-ID: <20111101152148.GA5552@sigill.intra.peff.net>
In-Reply-To: <20111029124755.GE2345@goldbirke>

On Sat, Oct 29, 2011 at 02:47:55PM +0200, SZEDER Gábor wrote:

Show 5 quoted lines

> > Grep only looks in the current subdirectory for matches.
> 
> Unless the user explicitly specifies the path(s)...  But that comes at
> the end of the command line, so the completion script could have no
> idea about it at the time of 'git grep <TAB>'.

True. But that's a more general problem. You have a 'tags' file that presumably represents the working tree. But you are not necessarily grepping there. You might be grepping something related (e.g., what's in the index), which makes the tags file still a good guess.

But you might also be grepping some totally unrelated branch, in which case this is not helpful.

I tend to think that we are OK erring on the side of using the 'tags' file, even if it might be wrong, since otherwise we have nothing to tab-complete. When the user hits <Tab>, they are asking us to make our best guess, and if they know there is nothing to complete, they won't hit <Tab>. So it's not like we're hurting some existing workflow.

And in that sense, maybe we should just do the "search back up the working tree" thing. Sure, it may be return way more matches than are accurate, but even that's better than having no tab-completion at all.

Show 7 quoted lines

> > You don't want __gitdir here, but rather "git rev-parse --show-cdup".
> 
> Oh, yes, indeed.
> 
> But there was a point in using __gitdir() here: it respects the
> --git-dir= option.  Which brings up the question: what
> should 'git --git-dir=/some/where/.git grep <TAB>' offer?

I guess if core.worktree is set, it would look there instead (and ditto for specifying --work-tree on the command line). Handling those is such a corner case that I'm not too concerned if we don't. And if somebody really cares, they can fix the completion later to pick up magic like that from the early part of the command line. I don't see it as a blocker for an initial version of the patch.

Show 10 quoted lines

> _get_comp_words_by_ref() is a general completion function, the purpose
> of which is to provide a bash-version-independent equivalent of
> $COMP_WORDS and $COMP_CWORD by working around the different word
> splitting rules.  It doesn't know about git and its commands at all.
> 
> But there is the while loop in _git() that looks for the git command
> (among other things) on the command line, which could store the index
> of the command name in $words in a variable.  This variable could then
> be used in that case statement (and probably in a couple of other
> places, too).

Yeah, that makes sense. Again, my inclination is to just leave that for a further patch if somebody really wants to make the completion (for this and any other positional slots) more accurate.

-Peff