threads / rfc / 20067

RFC patchgrep: Add --directories option.

Subject: [PATCH/RFC] grep: Add --directories option.

## tl;dr

6 messages between Jul 9, 2009 and Jul 10, 2009. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Michał Kiedrowicz· Jul 9, 2009, 19:20 UTC · lore

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 GNU grep compatible option "--directories=action" to git-grep. Currently supported actions are: recurse (default) and skip. Action 'read' is not implemented.

Documentation updates and simple test cases are also provided.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
 Documentation/git-grep.txt |    7 ++++
 builtin-grep.c             |   71 +++++++++++++++++++++++++++++++++----------
 t/t7002-grep.sh            |   34 ++++++++++++++++++++-
 3 files changed, 94 insertions(+), 18 deletions(-)
Show changes to 3 files +94 −18

Documentation/git-grep.txt, builtin-grep.c, t/t7002-grep.sh

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index fccb82d..1c4b1ff 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]
+	   [(-d|--directories) <action>]
 	   [--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.
 
+-d <action>::
+--directories=<action>::
+	If an input file is a directory, use `action` to process it. If
+	`action` is recurse (default), read all files under each directory,
+	recursively. If `action` is skip, directories are skipped.
+
 -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 e558368..27330e8 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -45,27 +45,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] != '/')
@@ -307,7 +314,8 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)
 		strbuf_setlen(sb, sb->len - 1);
 }
 
-static int external_grep(struct grep_opt *opt, const char **paths, int cached)
+static int external_grep(struct grep_opt *opt, const char **paths, int cached,
+		int recurse)
 {
 	int i, nr, argc, hit, len, status;
 	const char *argv[MAXARGS+1];
@@ -403,7 +411,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, recurse))
 			continue;
 		name = ce->name;
 		if (name[0] == '-') {
@@ -437,7 +445,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 #endif
 
 static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
-		      int external_grep_allowed)
+		      int external_grep_allowed, int recurse)
 {
 	int hit = 0;
 	int nr;
@@ -450,7 +458,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
 	 * be a lot more optimized
 	 */
 	if (!cached && external_grep_allowed) {
-		hit = external_grep(opt, paths, cached);
+		hit = external_grep(opt, paths, cached, recurse);
 		if (hit >= 0)
 			return hit;
 	}
@@ -460,7 +468,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, recurse))
 			continue;
 		/*
 		 * If CE_VALID is on, we assume worktree file and its cache entry
@@ -488,7 +496,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
 
 static int grep_tree(struct grep_opt *opt, const char **paths,
 		     struct tree_desc *tree,
-		     const char *tree_name, const char *base)
+		     const char *tree_name, const char *base,
+		     int recurse)
 {
 	int len;
 	int hit = 0;
@@ -520,7 +529,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, recurse))
 			;
 		else if (S_ISREG(entry.mode))
 			hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);
@@ -535,7 +544,8 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
 				die("unable to read tree (%s)",
 				    sha1_to_hex(entry.sha1));
 			init_tree_desc(&sub, data, size);
-			hit |= grep_tree(opt, paths, &sub, tree_name, down);
+			hit |= grep_tree(opt, paths, &sub, tree_name, down,
+					recurse);
 			free(data);
 		}
 	}
@@ -544,7 +554,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
 }
 
 static int grep_object(struct grep_opt *opt, const char **paths,
-		       struct object *obj, const char *name)
+		       struct object *obj, const char *name, int recurse)
 {
 	if (obj->type == OBJ_BLOB)
 		return grep_sha1(opt, obj->sha1, name, 0);
@@ -558,7 +568,7 @@ static int grep_object(struct grep_opt *opt, const char **paths,
 		if (!data)
 			die("unable to read tree (%s)", sha1_to_hex(obj->sha1));
 		init_tree_desc(&tree, data, size);
-		hit = grep_tree(opt, paths, &tree, name, "");
+		hit = grep_tree(opt, paths, &tree, name, "", recurse);
 		free(data);
 		return hit;
 	}
@@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
 	return -1;
 }
 
+static int directories_callback(const struct option *opt,
+				const char *arg, int unset)
+{
+	int *recurse = opt->value;
+
+	if (!arg)
+		return error("switch `d' requires a value");
+
+	if (!strcmp(arg, "recurse")) {
+		*recurse = 1;
+		return 0;
+	} else if (!strcmp(arg, "skip")) {
+		*recurse = 0;
+		return 0;
+	}
+
+	fprintf(stderr, "Invalid action `%s'.\n", arg);
+	fprintf(stderr, "Available actions are: recurse skip.\n");
+	return -1;
+}
+
 int cmd_grep(int argc, const char **argv, const char *prefix)
 {
 	int hit = 0;
 	int cached = 0;
+	int recurse = 1;
 	int external_grep_allowed = 1;
 	int seen_dashdash = 0;
 	struct grep_opt opt;
@@ -674,6 +706,9 @@ 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_CALLBACK('d', "directories", &recurse, "action",
+			"action to perform when input file is a directory",
+			directories_callback),
 		OPT_GROUP(""),
 		OPT_BIT('E', "extended-regexp", &opt.regflags,
 			"use extended POSIX regular expressions", REG_EXTENDED),
@@ -830,7 +865,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!list.nr) {
 		if (!cached)
 			setup_work_tree();
-		return !grep_cache(&opt, paths, cached, external_grep_allowed);
+		return !grep_cache(&opt, paths, cached, external_grep_allowed,
+				recurse);
 	}
 
 	if (cached)
@@ -839,7 +875,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < list.nr; i++) {
 		struct object *real_obj;
 		real_obj = deref_tag(list.objects[i].item, NULL, 0);
-		if (grep_object(&opt, paths, real_obj, list.objects[i].name))
+		if (grep_object(&opt, paths, real_obj, list.objects[i].name,
+					recurse))
 			hit = 1;
 	}
 	free_grep_patterns(&opt);
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 7868af8..6d1faf4 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -22,7 +22,9 @@ test_expect_success setup '
 	echo zzz > z &&
 	mkdir t &&
 	echo test >t/t &&
-	git add file w x y z t/t &&
+	mkdir t/a &&
+	echo aa aa aa aa >t/a/a &&
+	git add file w x y z t/t t/a/a &&
 	test_tick &&
 	git commit -m initial
 '
@@ -123,6 +125,36 @@ do
 		! git grep -c test $H | grep /dev/null
         '
 
+	test_expect_success "grep -d recurse $L" '
+		echo "${HC}t/t:1:test" >expected &&
+		git grep -d recurse -n -e test $H >actual &&
+		diff expected actual
+	'
+
+	test_expect_success "grep -d skip $L" '
+		: >expected &&
+		if git grep -d skip -e test $H >actual
+		then
+			echo should not have matched
+			cat actual
+			false
+		else
+			diff expected actual
+		fi
+	'
+
+	test_expect_success "grep -d skip $L -- t" '
+		: >expected &&
+		if git grep -d skip -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
René Scharfe· Jul 10, 2009, 07:33 UTC · re: Michał Kiedrowicz · lore

Re: [PATCH/RFC] grep: Add --directories option.

Michał Kiedrowicz schrieb:
Show 38 quoted lines
> 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 GNU grep compatible option
> "--directories=action" to git-grep. Currently supported actions are:
> recurse (default) and skip. Action 'read' is not implemented.
> 
> Documentation updates and simple test cases are also provided.
> 
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
> ---
>  Documentation/git-grep.txt |    7 ++++
>  builtin-grep.c             |   71 +++++++++++++++++++++++++++++++++----------
>  t/t7002-grep.sh            |   34 ++++++++++++++++++++-
>  3 files changed, 94 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index fccb82d..1c4b1ff 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]
> +	   [(-d|--directories) <action>]
>  	   [--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.
>  
> +-d <action>::
> +--directories=<action>::
> +	If an input file is a directory, use `action` to process it. If
> +	`action` is recurse (default), read all files under each directory,
> +	recursively. If `action` is skip, directories are skipped.
Perhaps quote 'recurse' and 'skip'?
Show 171 quoted lines
> +
>  -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 e558368..27330e8 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -45,27 +45,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] != '/')
> @@ -307,7 +314,8 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)
>  		strbuf_setlen(sb, sb->len - 1);
>  }
>  
> -static int external_grep(struct grep_opt *opt, const char **paths, int cached)
> +static int external_grep(struct grep_opt *opt, const char **paths, int cached,
> +		int recurse)
>  {
>  	int i, nr, argc, hit, len, status;
>  	const char *argv[MAXARGS+1];
> @@ -403,7 +411,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, recurse))
>  			continue;
>  		name = ce->name;
>  		if (name[0] == '-') {
> @@ -437,7 +445,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
>  #endif
>  
>  static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
> -		      int external_grep_allowed)
> +		      int external_grep_allowed, int recurse)
>  {
>  	int hit = 0;
>  	int nr;
> @@ -450,7 +458,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
>  	 * be a lot more optimized
>  	 */
>  	if (!cached && external_grep_allowed) {
> -		hit = external_grep(opt, paths, cached);
> +		hit = external_grep(opt, paths, cached, recurse);
>  		if (hit >= 0)
>  			return hit;
>  	}
> @@ -460,7 +468,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, recurse))
>  			continue;
>  		/*
>  		 * If CE_VALID is on, we assume worktree file and its cache entry
> @@ -488,7 +496,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
>  
>  static int grep_tree(struct grep_opt *opt, const char **paths,
>  		     struct tree_desc *tree,
> -		     const char *tree_name, const char *base)
> +		     const char *tree_name, const char *base,
> +		     int recurse)
>  {
>  	int len;
>  	int hit = 0;
> @@ -520,7 +529,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, recurse))
>  			;
>  		else if (S_ISREG(entry.mode))
>  			hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);
> @@ -535,7 +544,8 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
>  				die("unable to read tree (%s)",
>  				    sha1_to_hex(entry.sha1));
>  			init_tree_desc(&sub, data, size);
> -			hit |= grep_tree(opt, paths, &sub, tree_name, down);
> +			hit |= grep_tree(opt, paths, &sub, tree_name, down,
> +					recurse);
>  			free(data);
>  		}
>  	}
> @@ -544,7 +554,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
>  }
>  
>  static int grep_object(struct grep_opt *opt, const char **paths,
> -		       struct object *obj, const char *name)
> +		       struct object *obj, const char *name, int recurse)
>  {
>  	if (obj->type == OBJ_BLOB)
>  		return grep_sha1(opt, obj->sha1, name, 0);
> @@ -558,7 +568,7 @@ static int grep_object(struct grep_opt *opt, const char **paths,
>  		if (!data)
>  			die("unable to read tree (%s)", sha1_to_hex(obj->sha1));
>  		init_tree_desc(&tree, data, size);
> -		hit = grep_tree(opt, paths, &tree, name, "");
> +		hit = grep_tree(opt, paths, &tree, name, "", recurse);
>  		free(data);
>  		return hit;
>  	}
> @@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
>  	return -1;
>  }
>  
> +static int directories_callback(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	int *recurse = opt->value;
> +
> +	if (!arg)
> +		return error("switch `d' requires a value");
> +
> +	if (!strcmp(arg, "recurse")) {
> +		*recurse = 1;
> +		return 0;
> +	} else if (!strcmp(arg, "skip")) {
> +		*recurse = 0;
> +		return 0;
> +	}
> +
> +	fprintf(stderr, "Invalid action `%s'.\n", arg);
> +	fprintf(stderr, "Available actions are: recurse skip.\n");
> +	return -1;
> +}
> +
>  int cmd_grep(int argc, const char **argv, const char *prefix)
>  {
>  	int hit = 0;
>  	int cached = 0;
> +	int recurse = 1;

I suspect the patch would shrink significantly if you moved "int recurse" into struct grep_opt, because then you wouln't need to add it as a parameter to the grep_* functions.

Show 12 quoted lines
> diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> index 7868af8..6d1faf4 100755
> --- a/t/t7002-grep.sh
> +++ b/t/t7002-grep.sh
> @@ -22,7 +22,9 @@ test_expect_success setup '
>  	echo zzz > z &&
>  	mkdir t &&
>  	echo test >t/t &&
> -	git add file w x y z t/t &&
> +	mkdir t/a &&
> +	echo aa aa aa aa >t/a/a &&
> +	git add file w x y z t/t t/a/a &&
This conflicts with a recent change.
It seems your patch still allows recursion, one level deep.  In git's repo:
	$ grep -l --directories=skip GNU compat
	$ grep -l --directories=skip GNU compat/*
	compat/qsort.c
	compat/snprintf.c
	$ git grep -l --directories=skip GNU compat
	compat/qsort.c
	compat/snprintf.c
	$ git grep -l --directories=skip GNU compat/*
	compat/fnmatch/fnmatch.c
	compat/fnmatch/fnmatch.h
	compat/nedmalloc/malloc.c.h
	compat/nedmalloc/nedmalloc.c
	compat/nedmalloc/nedmalloc.h
	compat/qsort.c
	compat/regex/regex.c
	compat/regex/regex.h
	compat/snprintf.c
René
Junio C Hamano· Jul 10, 2009, 08:11 UTC · re: René Scharfe · lore

Re: [PATCH/RFC] grep: Add --directories option.

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> It seems your patch still allows recursion, one level deep.
I suspect what the patch wants to do may be fundamentally unworkable.

Unlike GNU grep that takes its command line arguments literally as files and directories, we use them merely as pathspec filters, so...

> 	$ git grep -l --directories=skip GNU compat/*

... while I think you should be able to compensate for this kind of "off-by-one" and make it appear to work, I do not think there is a good definition of which level it should stop if you run it with something like this (notice the single-quote around the pathspec to prevent it from getting expanded by the shell):

	git grep GNU 'compat/*/*'
Michał Kiedrowicz· Jul 10, 2009, 16:48 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] grep: Add --directories option.

Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
> > It seems your patch still allows recursion, one level deep.
> 
> I suspect what the patch wants to do may be fundamentally unworkable.
> 
> Unlike GNU grep that takes its command line arguments literally as
> files and directories, we use them merely as pathspec filters, so...
> 
> > 	$ git grep -l --directories=skip GNU compat/*
> 
> ... while I think you should be able to compensate for this kind  of
> "off-by-one" and make it appear to work, I do not think there is a
> good definition of which level it should stop if you run it with
> something like this (notice the single-quote around the pathspec to
> prevent it from getting expanded by the shell):
> 
> 	git grep GNU 'compat/*/*'
>

To makes things simplier (but maybe not correct) I assumed that '*' matches _any_ string, including '/', so any file in "compat/" (even in subdirs) should be treated as a file specified on command line.

Michał Kiedrowicz
Michał Kiedrowicz· Jul 10, 2009, 16:41 UTC · re: René Scharfe · lore

Re: [PATCH/RFC] grep: Add --directories option.

Hi,
René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> 
> I suspect the patch would shrink significantly if you moved "int
> recurse" into struct grep_opt, because then you wouln't need to add it
> as a parameter to the grep_* functions.

Yes. At the beginning, I placed 'int recurse' in that struct. However, I thought that this struct does not say anything about selecting files, but it says how to grep files (invert, count, fixed [string], binary etc.). Most of opts (if not all) are used in grep.c, not builtin-grep.c.

Show 15 quoted lines
> 
> > diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> > index 7868af8..6d1faf4 100755
> > --- a/t/t7002-grep.sh
> > +++ b/t/t7002-grep.sh
> > @@ -22,7 +22,9 @@ test_expect_success setup '
> >  	echo zzz > z &&
> >  	mkdir t &&
> >  	echo test >t/t &&
> > -	git add file w x y z t/t &&
> > +	mkdir t/a &&
> > +	echo aa aa aa aa >t/a/a &&
> > +	git add file w x y z t/t t/a/a &&
> 
> This conflicts with a recent change.
Yeah, I found out that...
Show 26 quoted lines
> 
> It seems your patch still allows recursion, one level deep.  In git's
> repo:
> 
> 	$ grep -l --directories=skip GNU compat
> 
> 	$ grep -l --directories=skip GNU compat/*
> 	compat/qsort.c
> 	compat/snprintf.c
> 
> 	$ git grep -l --directories=skip GNU compat
> 	compat/qsort.c
> 	compat/snprintf.c
> 
> 	$ git grep -l --directories=skip GNU compat/*
> 	compat/fnmatch/fnmatch.c
> 	compat/fnmatch/fnmatch.h
> 	compat/nedmalloc/malloc.c.h
> 	compat/nedmalloc/nedmalloc.c
> 	compat/nedmalloc/nedmalloc.h
> 	compat/qsort.c
> 	compat/regex/regex.c
> 	compat/regex/regex.h
> 	compat/snprintf.c
> 
> René

Actually, this is what I wanted: Do not descend to subdirectories of selected directories. After a while, I think this action should be called "read [files in that directory]", not "skip".

Stephen Boyd· Jul 10, 2009, 08:02 UTC · re: Michał Kiedrowicz · lore

Re: [PATCH/RFC] grep: Add --directories option.

Michał Kiedrowicz wrote:
Show 11 quoted lines
> @@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
>  	return -1;
>  }
>  
> +static int directories_callback(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	int *recurse = opt->value;
> +
> +	if (!arg)
> +		return error("switch `d' requires a value");
This isn't needed because OPT_CALLBACK requires an argument always be given.
Show 14 quoted lines
> +
> +	if (!strcmp(arg, "recurse")) {
> +		*recurse = 1;
> +		return 0;
> +	} else if (!strcmp(arg, "skip")) {
> +		*recurse = 0;
> +		return 0;
> +	}
> +
> +	fprintf(stderr, "Invalid action `%s'.\n", arg);
> +	fprintf(stderr, "Available actions are: recurse skip.\n");
> +	return -1;
> +}
> +

I think I would drop the two fprintf's here and just return an error() saying invalid action. This will in turn cause the usage message to show up, where you can show the two possible actions.

Show 10 quoted lines
> @@ -674,6 +706,9 @@ 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_CALLBACK('d', "directories", &recurse, "action",
> +			"action to perform when input file is a directory",
> +			directories_callback),
>  		OPT_GROUP(""),
>  		OPT_BIT('E', "extended-regexp", &opt.regflags,
>  			"use extended POSIX regular expressions", REG_EXTENDED),

Do you want to allow "--no-directories", I would suggest setting the PARSE_OPT_NONEG flag to avoid this. Maybe you want to replace "action" with "recurse|skip" too.

← back to recent threads