All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFC] grep: Add --directories option.
@ 2009-07-09 19:20 Michał Kiedrowicz
  2009-07-10  7:33 ` René Scharfe
  2009-07-10  8:02 ` Stephen Boyd
  0 siblings, 2 replies; 6+ messages in thread
From: Michał Kiedrowicz @ 2009-07-09 19:20 UTC (permalink / raw)
  To: Git Mailing List; +Cc: 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 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.
+
 -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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] grep: Add --directories option.
  2009-07-09 19:20 [PATCH/RFC] grep: Add --directories option Michał Kiedrowicz
@ 2009-07-10  7:33 ` René Scharfe
  2009-07-10  8:11   ` Junio C Hamano
  2009-07-10 16:41   ` Michał Kiedrowicz
  2009-07-10  8:02 ` Stephen Boyd
  1 sibling, 2 replies; 6+ messages in thread
From: René Scharfe @ 2009-07-10  7:33 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: Git Mailing List

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 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'?

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] grep: Add --directories option.
  2009-07-09 19:20 [PATCH/RFC] grep: Add --directories option Michał Kiedrowicz
  2009-07-10  7:33 ` René Scharfe
@ 2009-07-10  8:02 ` Stephen Boyd
  1 sibling, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2009-07-10  8:02 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: Git Mailing List

Michał Kiedrowicz wrote:
> @@ -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.

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] grep: Add --directories option.
  2009-07-10  7:33 ` René Scharfe
@ 2009-07-10  8:11   ` Junio C Hamano
  2009-07-10 16:48     ` Michał Kiedrowicz
  2009-07-10 16:41   ` Michał Kiedrowicz
  1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2009-07-10  8:11 UTC (permalink / raw)
  To: René Scharfe; +Cc: Michał Kiedrowicz, Git Mailing List

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/*/*'

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] grep: Add --directories option.
  2009-07-10  7:33 ` René Scharfe
  2009-07-10  8:11   ` Junio C Hamano
@ 2009-07-10 16:41   ` Michał Kiedrowicz
  1 sibling, 0 replies; 6+ messages in thread
From: Michał Kiedrowicz @ 2009-07-10 16:41 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List

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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] grep: Add --directories option.
  2009-07-10  8:11   ` Junio C Hamano
@ 2009-07-10 16:48     ` Michał Kiedrowicz
  0 siblings, 0 replies; 6+ messages in thread
From: Michał Kiedrowicz @ 2009-07-10 16:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, Git Mailing List

Junio C Hamano <gitster@pobox.com> wrote:

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-07-10 16:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-09 19:20 [PATCH/RFC] grep: Add --directories option Michał Kiedrowicz
2009-07-10  7:33 ` René Scharfe
2009-07-10  8:11   ` Junio C Hamano
2009-07-10 16:48     ` Michał Kiedrowicz
2009-07-10 16:41   ` Michał Kiedrowicz
2009-07-10  8:02 ` Stephen Boyd

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.