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

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

Hi,

On Fri, Oct 21, 2011 at 01:30:21PM -0400, Jeff King wrote:
> This incorporates the suggestions from Gábor's review, with one
> exception: it still looks only in the current directory for the "tags"
> files. I think that might have some performance implications, so I'd
> rather add it separately, if at all.

I agree that scanning through a whole working tree for tags files
would cost too much.  But I think that a tags file at the top of the
working tree is common enough to be supported, and checking its
existence is fairly cheap.

> +	case "$cword,$prev" in
> +	2,*|*,-*)
> +		if test -r tags; then
> +			__gitcomp "$(__git_match_ctag "$cur" tags)"
> +			return
> +		fi
> +		;;

So how about something like this for the case arm? (I didn't actually
tested it.)

		local tagsfile
		if test -r tags; then
			tagsfile=tags
		else
			local dir="$(__gitdir)"
			if test -r "$dir"/tags; then
				tagsfile="$dir"/tags
			fi
		fi
		if [ -n "tagsfile" ]; then
			__gitcomp "$(__git_match_ctag "$cur" "$tagsfile")"
			return
		fi


Btw, there is a bug in the case statement: 'git --no-pager grep <TAB>'
offers refs instead of symbols, because $cword is not 2 and $prev
doesn't start with a dash.  But it's not worse than the current
behavior, so I don't think this bug is a show-stopper for the patch.


Best,
Gábor