{"thread":{"id":"20067","subject":"[PATCH/RFC] grep: Add --directories option.","startedAt":"2009-07-09T19:20:28Z","lastAt":"2009-07-10T16:48:28Z","messageCount":6,"participants":["Michał Kiedrowicz","René Scharfe","Stephen Boyd","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"117718","messageId":"1247167228-2466-1-git-send-email-michal.kiedrowicz@gmail.com","threadId":"20067","inReplyTo":null,"subject":"[PATCH/RFC] grep: Add --directories option.","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-09T19:20:28Z","receivedAt":"2009-07-09T19:20:28Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Sometimes it is useful to grep directories non-recursive. E.g. if I want\nto look for all files in main directory, but not in any subdirectory.\nOr in Documentation/, but not in Documentation/technical/ and so on.\n\nThis patch adds support for GNU grep compatible option\n\"--directories=action\" to git-grep. Currently supported actions are:\nrecurse (default) and skip. Action 'read' is not implemented.\n\nDocumentation updates and simple test cases are also provided.\n\nSigned-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>\n---\n Documentation/git-grep.txt |    7 ++++\n builtin-grep.c             |   71 +++++++++++++++++++++++++++++++++----------\n t/t7002-grep.sh            |   34 ++++++++++++++++++++-\n 3 files changed, 94 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex fccb82d..1c4b1ff 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -17,6 +17,7 @@ SYNOPSIS\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match]\n+\t   [(-d|--directories) <action>]\n \t   [--color | --no-color]\n \t   [-A <post-context>] [-B <pre-context>] [-C <context>]\n \t   [-f <file>] [-e] <pattern>\n@@ -47,6 +48,12 @@ OPTIONS\n -I::\n \tDon't match the pattern in binary files.\n \n+-d <action>::\n+--directories=<action>::\n+\tIf an input file is a directory, use `action` to process it. If\n+\t`action` is recurse (default), read all files under each directory,\n+\trecursively. If `action` is skip, directories are skipped.\n+\n -w::\n --word-regexp::\n \tMatch the pattern only at word boundary (either begin at the\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex e558368..27330e8 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -45,27 +45,34 @@ static int grep_config(const char *var, const char *value, void *cb)\n \treturn git_color_default_config(var, value, cb);\n }\n \n+static inline int accept_subdir(const char *path, int recurse)\n+{\n+\treturn recurse || !strchr(path, '/');\n+}\n+\n /*\n  * git grep pathspecs are somewhat different from diff-tree pathspecs;\n  * pathname wildcards are allowed.\n  */\n-static int pathspec_matches(const char **paths, const char *name)\n+static int pathspec_matches(const char **paths, const char *name, int recurse)\n {\n \tint namelen, i;\n \tif (!paths || !*paths)\n-\t\treturn 1;\n+\t\treturn accept_subdir(name, recurse);\n \tnamelen = strlen(name);\n \tfor (i = 0; paths[i]; i++) {\n \t\tconst char *match = paths[i];\n \t\tint matchlen = strlen(match);\n \t\tconst char *cp, *meta;\n \n-\t\tif (!matchlen ||\n+\t\tif ((!matchlen && accept_subdir(name, recurse)) ||\n \t\t    ((matchlen <= namelen) &&\n \t\t     !strncmp(name, match, matchlen) &&\n-\t\t     (match[matchlen-1] == '/' ||\n-\t\t      name[matchlen] == '\\0' || name[matchlen] == '/')))\n+\t\t     (name[matchlen] == '\\0' ||\n+\t\t       ((match[matchlen-1] == '/'|| name[matchlen] == '/') &&\n+\t\t\taccept_subdir(name + matchlen + 1, recurse))))) {\n \t\t\treturn 1;\n+\t\t}\n \t\tif (!fnmatch(match, name, 0))\n \t\t\treturn 1;\n \t\tif (name[namelen-1] != '/')\n@@ -307,7 +314,8 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)\n \t\tstrbuf_setlen(sb, sb->len - 1);\n }\n \n-static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n+static int external_grep(struct grep_opt *opt, const char **paths, int cached,\n+\t\tint recurse)\n {\n \tint i, nr, argc, hit, len, status;\n \tconst char *argv[MAXARGS+1];\n@@ -403,7 +411,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n \t\tint kept;\n \t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n-\t\tif (!pathspec_matches(paths, ce->name))\n+\t\tif (!pathspec_matches(paths, ce->name, recurse))\n \t\t\tcontinue;\n \t\tname = ce->name;\n \t\tif (name[0] == '-') {\n@@ -437,7 +445,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n #endif\n \n static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n-\t\t      int external_grep_allowed)\n+\t\t      int external_grep_allowed, int recurse)\n {\n \tint hit = 0;\n \tint nr;\n@@ -450,7 +458,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n \t * be a lot more optimized\n \t */\n \tif (!cached && external_grep_allowed) {\n-\t\thit = external_grep(opt, paths, cached);\n+\t\thit = external_grep(opt, paths, cached, recurse);\n \t\tif (hit >= 0)\n \t\t\treturn hit;\n \t}\n@@ -460,7 +468,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n \t\tstruct cache_entry *ce = active_cache[nr];\n \t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n-\t\tif (!pathspec_matches(paths, ce->name))\n+\t\tif (!pathspec_matches(paths, ce->name, recurse))\n \t\t\tcontinue;\n \t\t/*\n \t\t * If CE_VALID is on, we assume worktree file and its cache entry\n@@ -488,7 +496,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n \n static int grep_tree(struct grep_opt *opt, const char **paths,\n \t\t     struct tree_desc *tree,\n-\t\t     const char *tree_name, const char *base)\n+\t\t     const char *tree_name, const char *base,\n+\t\t     int recurse)\n {\n \tint len;\n \tint hit = 0;\n@@ -520,7 +529,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \t\t\tstrbuf_addch(&pathbuf, '/');\n \n \t\tdown = pathbuf.buf + tn_len;\n-\t\tif (!pathspec_matches(paths, down))\n+\t\tif (!pathspec_matches(paths, down, recurse))\n \t\t\t;\n \t\telse if (S_ISREG(entry.mode))\n \t\t\thit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);\n@@ -535,7 +544,8 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \t\t\t\tdie(\"unable to read tree (%s)\",\n \t\t\t\t    sha1_to_hex(entry.sha1));\n \t\t\tinit_tree_desc(&sub, data, size);\n-\t\t\thit |= grep_tree(opt, paths, &sub, tree_name, down);\n+\t\t\thit |= grep_tree(opt, paths, &sub, tree_name, down,\n+\t\t\t\t\trecurse);\n \t\t\tfree(data);\n \t\t}\n \t}\n@@ -544,7 +554,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n }\n \n static int grep_object(struct grep_opt *opt, const char **paths,\n-\t\t       struct object *obj, const char *name)\n+\t\t       struct object *obj, const char *name, int recurse)\n {\n \tif (obj->type == OBJ_BLOB)\n \t\treturn grep_sha1(opt, obj->sha1, name, 0);\n@@ -558,7 +568,7 @@ static int grep_object(struct grep_opt *opt, const char **paths,\n \t\tif (!data)\n \t\t\tdie(\"unable to read tree (%s)\", sha1_to_hex(obj->sha1));\n \t\tinit_tree_desc(&tree, data, size);\n-\t\thit = grep_tree(opt, paths, &tree, name, \"\");\n+\t\thit = grep_tree(opt, paths, &tree, name, \"\", recurse);\n \t\tfree(data);\n \t\treturn hit;\n \t}\n@@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)\n \treturn -1;\n }\n \n+static int directories_callback(const struct option *opt,\n+\t\t\t\tconst char *arg, int unset)\n+{\n+\tint *recurse = opt->value;\n+\n+\tif (!arg)\n+\t\treturn error(\"switch `d' requires a value\");\n+\n+\tif (!strcmp(arg, \"recurse\")) {\n+\t\t*recurse = 1;\n+\t\treturn 0;\n+\t} else if (!strcmp(arg, \"skip\")) {\n+\t\t*recurse = 0;\n+\t\treturn 0;\n+\t}\n+\n+\tfprintf(stderr, \"Invalid action `%s'.\\n\", arg);\n+\tfprintf(stderr, \"Available actions are: recurse skip.\\n\");\n+\treturn -1;\n+}\n+\n int cmd_grep(int argc, const char **argv, const char *prefix)\n {\n \tint hit = 0;\n \tint cached = 0;\n+\tint recurse = 1;\n \tint external_grep_allowed = 1;\n \tint seen_dashdash = 0;\n \tstruct grep_opt opt;\n@@ -674,6 +706,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('I', NULL, &opt.binary,\n \t\t\t\"don't match patterns in binary files\",\n \t\t\tGREP_BINARY_NOMATCH),\n+\t\tOPT_CALLBACK('d', \"directories\", &recurse, \"action\",\n+\t\t\t\"action to perform when input file is a directory\",\n+\t\t\tdirectories_callback),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BIT('E', \"extended-regexp\", &opt.regflags,\n \t\t\t\"use extended POSIX regular expressions\", REG_EXTENDED),\n@@ -830,7 +865,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!list.nr) {\n \t\tif (!cached)\n \t\t\tsetup_work_tree();\n-\t\treturn !grep_cache(&opt, paths, cached, external_grep_allowed);\n+\t\treturn !grep_cache(&opt, paths, cached, external_grep_allowed,\n+\t\t\t\trecurse);\n \t}\n \n \tif (cached)\n@@ -839,7 +875,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tfor (i = 0; i < list.nr; i++) {\n \t\tstruct object *real_obj;\n \t\treal_obj = deref_tag(list.objects[i].item, NULL, 0);\n-\t\tif (grep_object(&opt, paths, real_obj, list.objects[i].name))\n+\t\tif (grep_object(&opt, paths, real_obj, list.objects[i].name,\n+\t\t\t\t\trecurse))\n \t\t\thit = 1;\n \t}\n \tfree_grep_patterns(&opt);\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex 7868af8..6d1faf4 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -22,7 +22,9 @@ test_expect_success setup '\n \techo zzz > z &&\n \tmkdir t &&\n \techo test >t/t &&\n-\tgit add file w x y z t/t &&\n+\tmkdir t/a &&\n+\techo aa aa aa aa >t/a/a &&\n+\tgit add file w x y z t/t t/a/a &&\n \ttest_tick &&\n \tgit commit -m initial\n '\n@@ -123,6 +125,36 @@ do\n \t\t! git grep -c test $H | grep /dev/null\n         '\n \n+\ttest_expect_success \"grep -d recurse $L\" '\n+\t\techo \"${HC}t/t:1:test\" >expected &&\n+\t\tgit grep -d recurse -n -e test $H >actual &&\n+\t\tdiff expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -d skip $L\" '\n+\t\t: >expected &&\n+\t\tif git grep -d skip -e test $H >actual\n+\t\tthen\n+\t\t\techo should not have matched\n+\t\t\tcat actual\n+\t\t\tfalse\n+\t\telse\n+\t\t\tdiff expected actual\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"grep -d skip $L -- t\" '\n+\t\t: >expected &&\n+\t\tif git grep -d skip -e aa $H -- t >actual\n+\t\tthen\n+\t\t\techo should not have matched\n+\t\t\tcat actual\n+\t\t\tfalse\n+\t\telse\n+\t\t\tdiff expected actual\n+\t\tfi\n+\t'\n+\n done\n \n cat >expected <<EOF\n-- \n1.6.3.3\n"},{"id":"117756","messageId":"4A56EED7.9040008@lsrfire.ath.cx","threadId":"20067","inReplyTo":"1247167228-2466-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"Re: [PATCH/RFC] grep: Add --directories option.","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-07-10T07:33:43Z","receivedAt":"2009-07-10T07:33:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Michał Kiedrowicz schrieb:\n> Sometimes it is useful to grep directories non-recursive. E.g. if I want\n> to look for all files in main directory, but not in any subdirectory.\n> Or in Documentation/, but not in Documentation/technical/ and so on.\n> \n> This patch adds support for GNU grep compatible option\n> \"--directories=action\" to git-grep. Currently supported actions are:\n> recurse (default) and skip. Action 'read' is not implemented.\n> \n> Documentation updates and simple test cases are also provided.\n> \n> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>\n> ---\n>  Documentation/git-grep.txt |    7 ++++\n>  builtin-grep.c             |   71 +++++++++++++++++++++++++++++++++----------\n>  t/t7002-grep.sh            |   34 ++++++++++++++++++++-\n>  3 files changed, 94 insertions(+), 18 deletions(-)\n> \n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index fccb82d..1c4b1ff 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -17,6 +17,7 @@ SYNOPSIS\n>  \t   [-l | --files-with-matches] [-L | --files-without-match]\n>  \t   [-z | --null]\n>  \t   [-c | --count] [--all-match]\n> +\t   [(-d|--directories) <action>]\n>  \t   [--color | --no-color]\n>  \t   [-A <post-context>] [-B <pre-context>] [-C <context>]\n>  \t   [-f <file>] [-e] <pattern>\n> @@ -47,6 +48,12 @@ OPTIONS\n>  -I::\n>  \tDon't match the pattern in binary files.\n>  \n> +-d <action>::\n> +--directories=<action>::\n> +\tIf an input file is a directory, use `action` to process it. If\n> +\t`action` is recurse (default), read all files under each directory,\n> +\trecursively. If `action` is skip, directories are skipped.\n\nPerhaps quote 'recurse' and 'skip'?\n\n> +\n>  -w::\n>  --word-regexp::\n>  \tMatch the pattern only at word boundary (either begin at the\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index e558368..27330e8 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -45,27 +45,34 @@ static int grep_config(const char *var, const char *value, void *cb)\n>  \treturn git_color_default_config(var, value, cb);\n>  }\n>  \n> +static inline int accept_subdir(const char *path, int recurse)\n> +{\n> +\treturn recurse || !strchr(path, '/');\n> +}\n> +\n>  /*\n>   * git grep pathspecs are somewhat different from diff-tree pathspecs;\n>   * pathname wildcards are allowed.\n>   */\n> -static int pathspec_matches(const char **paths, const char *name)\n> +static int pathspec_matches(const char **paths, const char *name, int recurse)\n>  {\n>  \tint namelen, i;\n>  \tif (!paths || !*paths)\n> -\t\treturn 1;\n> +\t\treturn accept_subdir(name, recurse);\n>  \tnamelen = strlen(name);\n>  \tfor (i = 0; paths[i]; i++) {\n>  \t\tconst char *match = paths[i];\n>  \t\tint matchlen = strlen(match);\n>  \t\tconst char *cp, *meta;\n>  \n> -\t\tif (!matchlen ||\n> +\t\tif ((!matchlen && accept_subdir(name, recurse)) ||\n>  \t\t    ((matchlen <= namelen) &&\n>  \t\t     !strncmp(name, match, matchlen) &&\n> -\t\t     (match[matchlen-1] == '/' ||\n> -\t\t      name[matchlen] == '\\0' || name[matchlen] == '/')))\n> +\t\t     (name[matchlen] == '\\0' ||\n> +\t\t       ((match[matchlen-1] == '/'|| name[matchlen] == '/') &&\n> +\t\t\taccept_subdir(name + matchlen + 1, recurse))))) {\n>  \t\t\treturn 1;\n> +\t\t}\n>  \t\tif (!fnmatch(match, name, 0))\n>  \t\t\treturn 1;\n>  \t\tif (name[namelen-1] != '/')\n> @@ -307,7 +314,8 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)\n>  \t\tstrbuf_setlen(sb, sb->len - 1);\n>  }\n>  \n> -static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n> +static int external_grep(struct grep_opt *opt, const char **paths, int cached,\n> +\t\tint recurse)\n>  {\n>  \tint i, nr, argc, hit, len, status;\n>  \tconst char *argv[MAXARGS+1];\n> @@ -403,7 +411,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n>  \t\tint kept;\n>  \t\tif (!S_ISREG(ce->ce_mode))\n>  \t\t\tcontinue;\n> -\t\tif (!pathspec_matches(paths, ce->name))\n> +\t\tif (!pathspec_matches(paths, ce->name, recurse))\n>  \t\t\tcontinue;\n>  \t\tname = ce->name;\n>  \t\tif (name[0] == '-') {\n> @@ -437,7 +445,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n>  #endif\n>  \n>  static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n> -\t\t      int external_grep_allowed)\n> +\t\t      int external_grep_allowed, int recurse)\n>  {\n>  \tint hit = 0;\n>  \tint nr;\n> @@ -450,7 +458,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n>  \t * be a lot more optimized\n>  \t */\n>  \tif (!cached && external_grep_allowed) {\n> -\t\thit = external_grep(opt, paths, cached);\n> +\t\thit = external_grep(opt, paths, cached, recurse);\n>  \t\tif (hit >= 0)\n>  \t\t\treturn hit;\n>  \t}\n> @@ -460,7 +468,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n>  \t\tstruct cache_entry *ce = active_cache[nr];\n>  \t\tif (!S_ISREG(ce->ce_mode))\n>  \t\t\tcontinue;\n> -\t\tif (!pathspec_matches(paths, ce->name))\n> +\t\tif (!pathspec_matches(paths, ce->name, recurse))\n>  \t\t\tcontinue;\n>  \t\t/*\n>  \t\t * If CE_VALID is on, we assume worktree file and its cache entry\n> @@ -488,7 +496,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n>  \n>  static int grep_tree(struct grep_opt *opt, const char **paths,\n>  \t\t     struct tree_desc *tree,\n> -\t\t     const char *tree_name, const char *base)\n> +\t\t     const char *tree_name, const char *base,\n> +\t\t     int recurse)\n>  {\n>  \tint len;\n>  \tint hit = 0;\n> @@ -520,7 +529,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>  \t\t\tstrbuf_addch(&pathbuf, '/');\n>  \n>  \t\tdown = pathbuf.buf + tn_len;\n> -\t\tif (!pathspec_matches(paths, down))\n> +\t\tif (!pathspec_matches(paths, down, recurse))\n>  \t\t\t;\n>  \t\telse if (S_ISREG(entry.mode))\n>  \t\t\thit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);\n> @@ -535,7 +544,8 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>  \t\t\t\tdie(\"unable to read tree (%s)\",\n>  \t\t\t\t    sha1_to_hex(entry.sha1));\n>  \t\t\tinit_tree_desc(&sub, data, size);\n> -\t\t\thit |= grep_tree(opt, paths, &sub, tree_name, down);\n> +\t\t\thit |= grep_tree(opt, paths, &sub, tree_name, down,\n> +\t\t\t\t\trecurse);\n>  \t\t\tfree(data);\n>  \t\t}\n>  \t}\n> @@ -544,7 +554,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>  }\n>  \n>  static int grep_object(struct grep_opt *opt, const char **paths,\n> -\t\t       struct object *obj, const char *name)\n> +\t\t       struct object *obj, const char *name, int recurse)\n>  {\n>  \tif (obj->type == OBJ_BLOB)\n>  \t\treturn grep_sha1(opt, obj->sha1, name, 0);\n> @@ -558,7 +568,7 @@ static int grep_object(struct grep_opt *opt, const char **paths,\n>  \t\tif (!data)\n>  \t\t\tdie(\"unable to read tree (%s)\", sha1_to_hex(obj->sha1));\n>  \t\tinit_tree_desc(&tree, data, size);\n> -\t\thit = grep_tree(opt, paths, &tree, name, \"\");\n> +\t\thit = grep_tree(opt, paths, &tree, name, \"\", recurse);\n>  \t\tfree(data);\n>  \t\treturn hit;\n>  \t}\n> @@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)\n>  \treturn -1;\n>  }\n>  \n> +static int directories_callback(const struct option *opt,\n> +\t\t\t\tconst char *arg, int unset)\n> +{\n> +\tint *recurse = opt->value;\n> +\n> +\tif (!arg)\n> +\t\treturn error(\"switch `d' requires a value\");\n> +\n> +\tif (!strcmp(arg, \"recurse\")) {\n> +\t\t*recurse = 1;\n> +\t\treturn 0;\n> +\t} else if (!strcmp(arg, \"skip\")) {\n> +\t\t*recurse = 0;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tfprintf(stderr, \"Invalid action `%s'.\\n\", arg);\n> +\tfprintf(stderr, \"Available actions are: recurse skip.\\n\");\n> +\treturn -1;\n> +}\n> +\n>  int cmd_grep(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint hit = 0;\n>  \tint cached = 0;\n> +\tint recurse = 1;\n\nI suspect the patch would shrink significantly if you moved \"int\nrecurse\" into struct grep_opt, because then you wouln't need to add it\nas a parameter to the grep_* functions.\n\n> diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\n> index 7868af8..6d1faf4 100755\n> --- a/t/t7002-grep.sh\n> +++ b/t/t7002-grep.sh\n> @@ -22,7 +22,9 @@ test_expect_success setup '\n>  \techo zzz > z &&\n>  \tmkdir t &&\n>  \techo test >t/t &&\n> -\tgit add file w x y z t/t &&\n> +\tmkdir t/a &&\n> +\techo aa aa aa aa >t/a/a &&\n> +\tgit add file w x y z t/t t/a/a &&\n\nThis conflicts with a recent change.\n\nIt seems your patch still allows recursion, one level deep.  In git's repo:\n\n\t$ grep -l --directories=skip GNU compat\n\n\t$ grep -l --directories=skip GNU compat/*\n\tcompat/qsort.c\n\tcompat/snprintf.c\n\n\t$ git grep -l --directories=skip GNU compat\n\tcompat/qsort.c\n\tcompat/snprintf.c\n\n\t$ git grep -l --directories=skip GNU compat/*\n\tcompat/fnmatch/fnmatch.c\n\tcompat/fnmatch/fnmatch.h\n\tcompat/nedmalloc/malloc.c.h\n\tcompat/nedmalloc/nedmalloc.c\n\tcompat/nedmalloc/nedmalloc.h\n\tcompat/qsort.c\n\tcompat/regex/regex.c\n\tcompat/regex/regex.h\n\tcompat/snprintf.c\n\nRené\n"},{"id":"117758","messageId":"4A56F58E.1080907@gmail.com","threadId":"20067","inReplyTo":"1247167228-2466-1-git-send-email-michal.kiedrowicz@gmail.com","subject":"Re: [PATCH/RFC] grep: Add --directories option.","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-07-10T08:02:22Z","receivedAt":"2009-07-10T08:02:22Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Michał Kiedrowicz wrote:\n> @@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)\n>  \treturn -1;\n>  }\n>  \n> +static int directories_callback(const struct option *opt,\n> +\t\t\t\tconst char *arg, int unset)\n> +{\n> +\tint *recurse = opt->value;\n> +\n> +\tif (!arg)\n> +\t\treturn error(\"switch `d' requires a value\");\n\nThis isn't needed because OPT_CALLBACK requires an argument always be given.\n\n> +\n> +\tif (!strcmp(arg, \"recurse\")) {\n> +\t\t*recurse = 1;\n> +\t\treturn 0;\n> +\t} else if (!strcmp(arg, \"skip\")) {\n> +\t\t*recurse = 0;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tfprintf(stderr, \"Invalid action `%s'.\\n\", arg);\n> +\tfprintf(stderr, \"Available actions are: recurse skip.\\n\");\n> +\treturn -1;\n> +}\n> +\n\nI think I would drop the two fprintf's here and just return an error()\nsaying invalid action. This will in turn cause the usage message to show\nup, where you can show the two possible actions.\n\n> @@ -674,6 +706,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_SET_INT('I', NULL, &opt.binary,\n>  \t\t\t\"don't match patterns in binary files\",\n>  \t\t\tGREP_BINARY_NOMATCH),\n> +\t\tOPT_CALLBACK('d', \"directories\", &recurse, \"action\",\n> +\t\t\t\"action to perform when input file is a directory\",\n> +\t\t\tdirectories_callback),\n>  \t\tOPT_GROUP(\"\"),\n>  \t\tOPT_BIT('E', \"extended-regexp\", &opt.regflags,\n>  \t\t\t\"use extended POSIX regular expressions\", REG_EXTENDED),\n\nDo you want to allow \"--no-directories\", I would suggest setting the\nPARSE_OPT_NONEG flag to avoid this. Maybe you want to replace \"action\"\nwith \"recurse|skip\" too.\n"},{"id":"117759","messageId":"7vmy7dufdp.fsf@alter.siamese.dyndns.org","threadId":"20067","inReplyTo":"4A56EED7.9040008@lsrfire.ath.cx","subject":"Re: [PATCH/RFC] grep: Add --directories option.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-10T08:11:14Z","receivedAt":"2009-07-10T08:11:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> It seems your patch still allows recursion, one level deep.\n\nI suspect what the patch wants to do may be fundamentally unworkable.\n\nUnlike GNU grep that takes its command line arguments literally as files\nand directories, we use them merely as pathspec filters, so...\n\n> \t$ git grep -l --directories=skip GNU compat/*\n\n... while I think you should be able to compensate for this kind  of\n\"off-by-one\" and make it appear to work, I do not think there is a good\ndefinition of which level it should stop if you run it with something\nlike this (notice the single-quote around the pathspec to prevent it from\ngetting expanded by the shell):\n\n\tgit grep GNU 'compat/*/*'\n"},{"id":"117777","messageId":"20090710184119.072e5a0f@gmail.com","threadId":"20067","inReplyTo":"4A56EED7.9040008@lsrfire.ath.cx","subject":"Re: [PATCH/RFC] grep: Add --directories option.","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-10T16:41:19Z","receivedAt":"2009-07-10T16:41:19Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Hi,\n\nRené Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:\n\n> \n> I suspect the patch would shrink significantly if you moved \"int\n> recurse\" into struct grep_opt, because then you wouln't need to add it\n> as a parameter to the grep_* functions.\n\nYes. At the beginning, I placed 'int recurse' in that struct. However, I\nthought that this struct does not say anything about selecting files,\nbut it says how to grep files (invert, count, fixed [string], binary\netc.). Most of opts (if not all) are used in grep.c, not builtin-grep.c.\n\n> \n> > diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\n> > index 7868af8..6d1faf4 100755\n> > --- a/t/t7002-grep.sh\n> > +++ b/t/t7002-grep.sh\n> > @@ -22,7 +22,9 @@ test_expect_success setup '\n> >  \techo zzz > z &&\n> >  \tmkdir t &&\n> >  \techo test >t/t &&\n> > -\tgit add file w x y z t/t &&\n> > +\tmkdir t/a &&\n> > +\techo aa aa aa aa >t/a/a &&\n> > +\tgit add file w x y z t/t t/a/a &&\n> \n> This conflicts with a recent change.\n\nYeah, I found out that...\n> \n> It seems your patch still allows recursion, one level deep.  In git's\n> repo:\n> \n> \t$ grep -l --directories=skip GNU compat\n> \n> \t$ grep -l --directories=skip GNU compat/*\n> \tcompat/qsort.c\n> \tcompat/snprintf.c\n> \n> \t$ git grep -l --directories=skip GNU compat\n> \tcompat/qsort.c\n> \tcompat/snprintf.c\n> \n> \t$ git grep -l --directories=skip GNU compat/*\n> \tcompat/fnmatch/fnmatch.c\n> \tcompat/fnmatch/fnmatch.h\n> \tcompat/nedmalloc/malloc.c.h\n> \tcompat/nedmalloc/nedmalloc.c\n> \tcompat/nedmalloc/nedmalloc.h\n> \tcompat/qsort.c\n> \tcompat/regex/regex.c\n> \tcompat/regex/regex.h\n> \tcompat/snprintf.c\n> \n> René\n\nActually, this is what I wanted: Do not descend to subdirectories of\nselected directories. After a while, I think this action should be\ncalled \"read [files in that directory]\", not \"skip\".\n"},{"id":"117778","messageId":"20090710184828.63d123d0@gmail.com","threadId":"20067","inReplyTo":"7vmy7dufdp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] grep: Add --directories option.","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-10T16:48:28Z","receivedAt":"2009-07-10T16:48:28Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n\n> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n> \n> > It seems your patch still allows recursion, one level deep.\n> \n> I suspect what the patch wants to do may be fundamentally unworkable.\n> \n> Unlike GNU grep that takes its command line arguments literally as\n> files and directories, we use them merely as pathspec filters, so...\n> \n> > \t$ git grep -l --directories=skip GNU compat/*\n> \n> ... while I think you should be able to compensate for this kind  of\n> \"off-by-one\" and make it appear to work, I do not think there is a\n> good definition of which level it should stop if you run it with\n> something like this (notice the single-quote around the pathspec to\n> prevent it from getting expanded by the shell):\n> \n> \tgit grep GNU 'compat/*/*'\n> \n\nTo makes things simplier (but maybe not correct) I assumed that '*'\nmatches _any_ string, including '/', so any file in \"compat/\" (even\nin subdirs) should be treated as a file specified on command line.\n\nMichał Kiedrowicz\n"}]}