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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Oct 23, 2011, 21:29 UTC
Message-ID: <20111023212928.GG22551@goldbirke>
In-Reply-To: <20111021173021.GC24417@sigill.intra.peff.net>

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.

Show 7 quoted lines

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