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

[Jeff King <peff@peff.net>]

On Sun, Oct 23, 2011 at 11:29:28PM +0200, SZEDER Gábor wrote:

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

Actually, it's not too expensive. Asking git for the top of the working
tree means it has to traverse up to there anyway. So the trick is just
doing our search without invoking too many external tools which would
cause unnecessary forks.

The patch is below, but I'm still not sure it's a good idea.

Grep only looks in the current subdirectory for matches. So if I am in
"src/foo/bar/", and the tags file is in "src/", then is it really a good
way to generate completions? Most of what's in the tags file will
probably be in _other_ subdirectories of "src/", and won't be matched by
grep at all. So we will give useless entries to bash to complete.

> 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)"

You don't want __gitdir here, but rather "git rev-parse --show-cdup".

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

Yeah. The intent of the "$cword is 2" thing is to know that we are the
first non-option argument. Arguably, _get_comp_words_by_ref should
somehow tell us which position we are completing relative to the actual
command name.

-Peff

---
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index af283cb..b0ed657 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1429,6 +1429,39 @@ _git_gitk ()
 	_gitk
 }
 
+__git_cdup_dirs() {
+	local prefix=$(git rev-parse --show-cdup 2>/dev/null)
+	local oldifs=$IFS
+	local dots
+	local i
+	IFS=/
+	for i in $prefix; do
+		dots=$dots../
+		echo "$dots"
+	done
+	IFS=$oldifs
+}
+
+__git_find_in_cdup_one() {
+	local dir=$1; shift
+	for i in "$@"; do
+		if test -r "$dir$i"; then
+			echo "$dir$i"
+			return 0
+		fi
+	done
+	return 1
+}
+
+__git_find_in_cdup() {
+	__git_find_in_cdup_one "" "$@" && return
+
+	local dir
+	for dir in $(__git_cdup_dirs); do
+		__git_find_in_cdup_one "$dir" "$@" && return
+	done
+}
+
 __git_match_ctag() {
 	awk "/^${1////\\/}/ { print \$1 }" "$2"
 }
@@ -1457,8 +1490,9 @@ _git_grep ()
 
 	case "$cword,$prev" in
 	2,*|*,-*)
-		if test -r tags; then
-			__gitcomp "$(__git_match_ctag "$cur" tags)"
+		local tags=$(__git_find_in_cdup tags)
+		if test -n "$tags"; then
+			__gitcomp "$(__git_match_ctag "$cur" "$tags")"
 			return
 		fi
 		;;