[PATCH/RFC v2] grep: Add --[no-]recurse options.

[PATCH/RFC v2] grep: Add --[no-]recurse options.

[Michał Kiedrowicz]

Sometimes it is useful to grep directories non-recursive. E.g. if I want
to look for all files in main directory, but not in any subdirectory.
Or in Documentation/, but not in Documentation/technical/ and so on.

This patch adds support for --no-recurse and (for symmetry) --recurse
options to git-grep. If --no-recurse is set, git-grep does not descend
to subdirectories. When --recurse option is set, directories are
searched recursively (this is the default behavior).

If path specified on command line contains wildcards, option --no-recurse
makes no sense, i.e.

$ git grep -l --no-recurse GNU -- 'contrib/*'

(note the quotes) will search all files in contrib/, even in
subdirectories, because '*' matches all files.

Documentation updates, bash-completion and simple test cases are also
provided.

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---

Changes from previous version:
 * Renamed '--directories=action' to '--[no-]recurse', because my
   implementation is not compatible with GNU. Also parsing this option was
   simplified.
 * Moved 'int recurse' into 'struct grep_opt'.
 * Fixed merge conflict with 'master'.
 * Added bash-completion.

 Documentation/git-grep.txt             |    7 ++++++
 builtin-grep.c                         |   27 +++++++++++++++++-------
 contrib/completion/git-completion.bash |    1 +
 grep.h                                 |    1 +
 t/t7002-grep.sh                        |   34 +++++++++++++++++++++++++++++++-
 5 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index b753c9d..1766d33 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [-z | --null]
 	   [-c | --count] [--all-match]
+	   [--recurse | --no-recurse]
 	   [--color | --no-color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
@@ -47,6 +48,12 @@ OPTIONS
 -I::
 	Don't match the pattern in binary files.
 
+--recurse::
+	Process directories recursively. This is the default.
+
+--no-recurse::
+	Do not descend to subdirectories.
+
 -w::
 --word-regexp::
 	Match the pattern only at word boundary (either begin at the
diff --git a/builtin-grep.c b/builtin-grep.c
index f477659..28e980a 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -52,27 +52,34 @@ static int grep_config(const char *var, const char *value, void *cb)
 	return git_color_default_config(var, value, cb);
 }
 
+static inline int accept_subdir(const char *path, int recurse)
+{
+	return recurse || !strchr(path, '/');
+}
+
 /*
  * git grep pathspecs are somewhat different from diff-tree pathspecs;
  * pathname wildcards are allowed.
  */
-static int pathspec_matches(const char **paths, const char *name)
+static int pathspec_matches(const char **paths, const char *name, int recurse)
 {
 	int namelen, i;
 	if (!paths || !*paths)
-		return 1;
+		return accept_subdir(name, recurse);
 	namelen = strlen(name);
 	for (i = 0; paths[i]; i++) {
 		const char *match = paths[i];
 		int matchlen = strlen(match);
 		const char *cp, *meta;
 
-		if (!matchlen ||
+		if ((!matchlen && accept_subdir(name, recurse)) ||
 		    ((matchlen <= namelen) &&
 		     !strncmp(name, match, matchlen) &&
-		     (match[matchlen-1] == '/' ||
-		      name[matchlen] == '\0' || name[matchlen] == '/')))
+		     (name[matchlen] == '\0' ||
+		       ((match[matchlen-1] == '/'|| name[matchlen] == '/') &&
+			accept_subdir(name + matchlen + 1, recurse))))) {
 			return 1;
+		}
 		if (!fnmatch(match, name, 0))
 			return 1;
 		if (name[namelen-1] != '/')
@@ -421,7 +428,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		int kept;
 		if (!S_ISREG(ce->ce_mode))
 			continue;
-		if (!pathspec_matches(paths, ce->name))
+		if (!pathspec_matches(paths, ce->name, opt->recurse))
 			continue;
 		name = ce->name;
 		if (name[0] == '-') {
@@ -478,7 +485,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
 		struct cache_entry *ce = active_cache[nr];
 		if (!S_ISREG(ce->ce_mode))
 			continue;
-		if (!pathspec_matches(paths, ce->name))
+		if (!pathspec_matches(paths, ce->name, opt->recurse))
 			continue;
 		/*
 		 * If CE_VALID is on, we assume worktree file and its cache entry
@@ -538,7 +545,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
 			strbuf_addch(&pathbuf, '/');
 
 		down = pathbuf.buf + tn_len;
-		if (!pathspec_matches(paths, down))
+		if (!pathspec_matches(paths, down, opt->recurse))
 			;
 		else if (S_ISREG(entry.mode))
 			hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);
@@ -692,6 +699,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('I', NULL, &opt.binary,
 			"don't match patterns in binary files",
 			GREP_BINARY_NOMATCH),
+		OPT_BOOLEAN(0, "recurse", &opt.recurse,
+				"search directories recursively"),
 		OPT_GROUP(""),
 		OPT_BIT('E', "extended-regexp", &opt.regflags,
 			"use extended POSIX regular expressions", REG_EXTENDED),
@@ -775,6 +784,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (opt.color == -1)
 		opt.color = git_use_color_default;
 
+	opt.recurse = 1;
+
 	/*
 	 * If there is no -- then the paths must exist in the working
 	 * tree.  If there is no explicit pattern specified with -e or
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9c48864..5eec3cc 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1036,6 +1036,7 @@ _git_grep ()
 			--extended-regexp --basic-regexp --fixed-strings
 			--files-with-matches --name-only
 			--files-without-match
+			--recurse --no-recurse
 			--count
 			--and --or --not --all-match
 			"
diff --git a/grep.h b/grep.h
index f00db0e..6badf51 100644
--- a/grep.h
+++ b/grep.h
@@ -79,6 +79,7 @@ struct grep_opt {
 	int pathname;
 	int null_following_name;
 	int color;
+	int recurse;
 	int funcname;
 	char color_match[COLOR_MAXLEN];
 	const char *color_external;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index b13aa7e..0633673 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -31,7 +31,9 @@ test_expect_success setup '
 	echo zzz > z &&
 	mkdir t &&
 	echo test >t/t &&
-	git add file w x y z t/t hello.c &&
+	mkdir t/a &&
+	echo aa aa aa aa >t/a/a &&
+	git add file w x y z t/t t/a/a hello.c &&
 	test_tick &&
 	git commit -m initial
 '
@@ -132,6 +134,36 @@ do
 		! git grep -c test $H | grep /dev/null
         '
 
+	test_expect_success "grep --recurse $L" '
+		echo "${HC}t/t:1:test" >expected &&
+		git grep --recurse -n -e test $H >actual &&
+		diff expected actual
+	'
+
+	test_expect_success "grep --no-recurse $L" '
+		: >expected &&
+		if git grep --no-recurse -e test $H >actual
+		then
+			echo should not have matched
+			cat actual
+			false
+		else
+			diff expected actual
+		fi
+	'
+
+	test_expect_success "grep --no-recurse $L -- t" '
+		: >expected &&
+		if git grep --no-recurse -e aa $H -- t >actual
+		then
+			echo should not have matched
+			cat actual
+			false
+		else
+			diff expected actual
+		fi
+	'
+
 done
 
 cat >expected <<EOF
-- 
1.6.3.3

Re: [PATCH/RFC v2] grep: Add --[no-]recurse options.

[René Scharfe]

Michał Kiedrowicz schrieb:
> Sometimes it is useful to grep directories non-recursive. E.g. if I want
> to look for all files in main directory, but not in any subdirectory.
> Or in Documentation/, but not in Documentation/technical/ and so on.
> 
> This patch adds support for --no-recurse and (for symmetry) --recurse
> options to git-grep. If --no-recurse is set, git-grep does not descend
> to subdirectories. When --recurse option is set, directories are
> searched recursively (this is the default behavior).
> 
> If path specified on command line contains wildcards, option --no-recurse
> makes no sense, i.e.
> 
> $ git grep -l --no-recurse GNU -- 'contrib/*'
> 
> (note the quotes) will search all files in contrib/, even in
> subdirectories, because '*' matches all files.
>
> Documentation updates, bash-completion and simple test cases are also
> provided.
> 
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
> ---
> 
> Changes from previous version:
>  * Renamed '--directories=action' to '--[no-]recurse', because my
>    implementation is not compatible with GNU. Also parsing this option was
>    simplified.

OK.  Yet another idea: if we can't copy an option from grep, perhaps we
can steal it from another program?  Would find's -maxdepth option fit
the bill?  I imagine it git grep --max-depth=<n> counting the slashes in
path specs and files and rejecting those files whose count is higher
than the one of its (otherwise matching) pattern plus n.

> diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> index b13aa7e..0633673 100755
> --- a/t/t7002-grep.sh
> +++ b/t/t7002-grep.sh
> @@ -31,7 +31,9 @@ test_expect_success setup '
>  	echo zzz > z &&
>  	mkdir t &&
>  	echo test >t/t &&
> -	git add file w x y z t/t hello.c &&
> +	mkdir t/a &&
> +	echo aa aa aa aa >t/a/a &&
> +	git add file w x y z t/t t/a/a hello.c &&
>  	test_tick &&
>  	git commit -m initial
>  '
> @@ -132,6 +134,36 @@ do
>  		! git grep -c test $H | grep /dev/null
>          '
>  
> +	test_expect_success "grep --recurse $L" '
> +		echo "${HC}t/t:1:test" >expected &&
> +		git grep --recurse -n -e test $H >actual &&
> +		diff expected actual
> +	'

Use test_cmp instead of diff.

> +
> +	test_expect_success "grep --no-recurse $L" '
> +		: >expected &&
> +		if git grep --no-recurse -e test $H >actual
> +		then
> +			echo should not have matched
> +			cat actual
> +			false
> +		else
> +			diff expected actual
> +		fi
> +	'

Hmm, if git crashed and only wrote to stderr, this test would pass
(hint: try this with git's master, that doesn't know this option).
Better to devise a test in which the tested command still has to print
something (a restricted set of results in this case).

> +
> +	test_expect_success "grep --no-recurse $L -- t" '
> +		: >expected &&
> +		if git grep --no-recurse -e aa $H -- t >actual
> +		then
> +			echo should not have matched
> +			cat actual
> +			false
> +		else
> +			diff expected actual
> +		fi
> +	'

Same here.