{"thread":{"id":"20187","subject":"[PATCH v3] grep: Add --max-depth option.","startedAt":"2009-07-21T21:51:04Z","lastAt":"2009-07-22T17:08:44Z","messageCount":4,"participants":["Michał Kiedrowicz","Stephen Boyd","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"118430","messageId":"1248213064-1541-1-git-send-email-michal.kiedrowicz@gmail.com","threadId":"20187","inReplyTo":null,"subject":"[PATCH v3] grep: Add --max-depth option.","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-21T21:51:04Z","receivedAt":"2009-07-21T21:51:04Z","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 one\nwants to 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 --max-depth option to git-grep. If it is\nset, git-grep descends at most levels of directories below paths\nspecified on command line.\n\nNote that if path specified on command line contains wildcards, option\n--max-depth makes no sense, i.e.\n\n$ git grep -l --max-depth 0 GNU -- 'contrib/*'\n\n(note the quotes) will search all files in contrib/, even in\nsubdirectories, because '*' matches all files.\n\nDocumentation updates, bash-completion and simple test cases are also\nprovided.\n\nSigned-off-by: Michał Kiedrowicz \n---\n Documentation/git-grep.txt | 5 +++\n builtin-grep.c | 55 ++++++++++++++++++++++++++------\n contrib/completion/git-completion.bash | 1 +\n grep.h | 1 +\n t/t7002-grep.sh | 51 +++++++++++++++++++++++++++++-\n 5 files changed, 102 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex b753c9d..faa92d1 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 [--max-depth ]\n \t [--color | --no-color]\n \t [-A ] [-B ] [-C ]\n \t [-f ] [-e] \n@@ -47,6 +48,10 @@ OPTIONS\n -I::\n \tDon't match the pattern in binary files.\n \n+--max-depth ::\n+\tDescend at most levels of directories below paths specified on\n+\tcommand line. Negative value means no limit.\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 f477659..2ed2507 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -52,26 +52,58 @@ 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 count_chars(const char *str, char c)\n+{\n+\tint num = 0;\n+\n+\tif (!str)\n+\t\treturn 0;\n+\n+\tfor (; *str; str++)\n+\t\tif (*str == c)\n+\t\t\tnum++;\n+\treturn num;\n+}\n+\n+static inline int accept_subdir(const char *path, int max_depth)\n+{\n+\treturn max_depth < 0 || count_chars(path, '/') <= max_depth;\n+}\n+\n+/*\n+ * Return non-zero if name is a subdirectory of match and is not too deep.\n+ */\n+static inline int is_subdir(const char *name, int namelen,\n+\t\tconst char *match, int matchlen, int max_depth)\n+{\n+\tif (matchlen > namelen || strncmp(name, match, matchlen))\n+\t\treturn 0;\n+\n+\tif (name[matchlen] == '\\0') /* exact match */\n+\t\treturn 1;\n+\n+\tif (!matchlen || match[matchlen-1] == '/' || name[matchlen] == '/')\n+\t\treturn accept_subdir(name + matchlen + 1, max_depth);\n+\n+\treturn 0;\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 max_depth)\n {\n \tint namelen, i;\n \tif (!paths || !*paths)\n-\t\treturn 1;\n+\t\treturn accept_subdir(name, max_depth);\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\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\tif (is_subdir(name, namelen, match, matchlen, max_depth))\n \t\t\treturn 1;\n \t\tif (!fnmatch(match, name, 0))\n \t\t\treturn 1;\n@@ -421,7 +453,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, opt->max_depth))\n \t\t\tcontinue;\n \t\tname = ce->name;\n \t\tif (name[0] == '-') {\n@@ -478,7 +510,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, opt->max_depth))\n \t\t\tcontinue;\n \t\t/*\n \t\t * If CE_VALID is on, we assume worktree file and its cache entry\n@@ -538,7 +570,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, opt->max_depth))\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@@ -692,6 +724,8 @@ 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_INTEGER(0, \"max-depth\", &opt.max_depth,\n+\t\t\t\t\"descend at most levels\"),\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@@ -768,6 +802,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \topt.pathname = 1;\n \topt.pattern_tail = &opt.pattern_list;\n \topt.regflags = REG_NEWLINE;\n+\topt.max_depth = -1;\n \n \tstrcpy(opt.color_match, GIT_COLOR_RED GIT_COLOR_BOLD);\n \topt.color = -1;\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 887731e..fb05c48 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1036,6 +1036,7 @@ _git_grep ()\n \t\t\t--extended-regexp --basic-regexp --fixed-strings\n \t\t\t--files-with-matches --name-only\n \t\t\t--files-without-match\n+\t\t\t--max-depth\n \t\t\t--count\n \t\t\t--and --or --not --all-match\n \t\t\t\"\ndiff --git a/grep.h b/grep.h\nindex f00db0e..28e6b2a 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -79,6 +79,7 @@ struct grep_opt {\n \tint pathname;\n \tint null_following_name;\n \tint color;\n+\tint max_depth;\n \tint funcname;\n \tchar color_match[COLOR_MAXLEN];\n \tconst char *color_external;\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex b13aa7e..b4709e2 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -25,13 +25,17 @@ test_expect_success setup '\n \t\techo foo mmap bar_mmap\n \t\techo foo_mmap bar mmap baz\n \t} >file &&\n+\techo vvv >v &&\n \techo ww w >w &&\n \techo x x xx x >x &&\n \techo y yy >y &&\n \techo zzz > z &&\n \tmkdir t &&\n \techo test >t/t &&\n-\tgit add file w x y z t/t hello.c &&\n+\techo vvv >t/v &&\n+\tmkdir t/a &&\n+\techo vvv >t/a/v &&\n+\tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n '\n@@ -132,6 +136,51 @@ do\n \t\t! git grep -c test $H | grep /dev/null\n '\n \n+\ttest_expect_success \"grep --max-depth -1 $L\" '\n+\t\t{\n+\t\t\techo ${HC}t/a/v:1:vvv\n+\t\t\techo ${HC}t/v:1:vvv\n+\t\t\techo ${HC}v:1:vvv\n+\t\t} >expected &&\n+\t\tgit grep --max-depth -1 -n -e vvv $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep --max-depth 0 $L\" '\n+\t\t{\n+\t\t\techo ${HC}v:1:vvv\n+\t\t} >expected &&\n+\t\tgit grep --max-depth 0 -n -e vvv $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep --max-depth 0 -- '*' $L\" '\n+\t\t{\n+\t\t\techo ${HC}t/a/v:1:vvv\n+\t\t\techo ${HC}t/v:1:vvv\n+\t\t\techo ${HC}v:1:vvv\n+\t\t} >expected &&\n+\t\tgit grep --max-depth 0 -n -e vvv $H -- \"*\" >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep --max-depth 1 $L\" '\n+\t\t{\n+\t\t\techo ${HC}t/v:1:vvv\n+\t\t\techo ${HC}v:1:vvv\n+\t\t} >expected &&\n+\t\tgit grep --max-depth 1 -n -e vvv $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep --max-depth 0 -- t $L\" '\n+\t\t{\n+\t\t\techo ${HC}t/v:1:vvv\n+\t\t} >expected &&\n+\t\tgit grep --max-depth 0 -n -e vvv $H -- t >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n done\n \n cat >expected <\n> Signed-off-by: Michał Kiedrowicz \n> ---\n\nWould have been nice to see a comment here saying what's changed since\nv2. Looks like you renamed it to --max-depth, added some new inline\nfunctions, and fixed up the test style issues?\n\n>\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index f477659..2ed2507 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -52,26 +52,58 @@ 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 count_chars(const char *str, char c)\n> +{\n> +\tint num = 0;\n> +\n> +\tif (!str)\n> +\t\treturn 0;\n> +\n> +\tfor (; *str; str++)\n> +\t\tif (*str == c)\n> +\t\t\tnum++;\n> +\treturn num;\n> +}\n> +\n> +static inline int accept_subdir(const char *path, int max_depth)\n> +{\n> +\treturn max_depth < 0 || count_chars(path, '/') <= max_depth;\n> +}\n> +\n\nIf count_chars() is only used by accept_subdir() why not just manually\ninline the loop in accept_subdir()? Plus, you rightly return early if\nmax_depth is -1, but what if max_depth is something small like 1? Then\nyou still count all the slashes when you could return upon seeing the\nsecond slash.\n\n>\n> @@ -692,6 +724,8 @@ 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_INTEGER(0, \"max-depth\", &opt.max_depth,\n> +\t\t\t\t\"descend at most levels\"),\n\nPlease use OPTION_INTEGER here so you can use \"\" instead of\n\"\". This will make it more consistent with the documentation.\n"},{"id":"118463","messageId":"20090722180038.16a76b95@gmail.com","threadId":"20187","inReplyTo":"4A66BB64.5070106@gmail.com","subject":"Re: [PATCH v3] grep: Add --max-depth option.","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-22T16:00:38Z","receivedAt":"2009-07-22T16:00:38Z","isPatch":true,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Stephen Boyd wrote:\n\n> Michał Kiedrowicz wrote:\n> >\n> > Signed-off-by: Michał Kiedrowicz \n> > ---\n> \n> Would have been nice to see a comment here saying what's changed since\n> v2. \n\nSorry, I did that when I sent v2, but this time when I remembered about\nti, it was already too late :)\n\n> Looks like you renamed it to --max-depth, added some new inline\n> functions, and fixed up the test style issues?\n> \n\nYes, I followed your and René's suggestions and added ability to\nspecify max number of levels.\n\n> >\n> > diff --git a/builtin-grep.c b/builtin-grep.c\n> > index f477659..2ed2507 100644\n> > --- a/builtin-grep.c\n> > +++ b/builtin-grep.c\n> > @@ -52,26 +52,58 @@ static int grep_config(const char *var, const\n> > char *value, void *cb) return git_color_default_config(var, value,\n> > cb); }\n> > \n> > +static inline int count_chars(const char *str, char c)\n> > +{\n> > +\tint num = 0;\n> > +\n> > +\tif (!str)\n> > +\t\treturn 0;\n> > +\n> > +\tfor (; *str; str++)\n> > +\t\tif (*str == c)\n> > +\t\t\tnum++;\n> > +\treturn num;\n> > +}\n> > +\n> > +static inline int accept_subdir(const char *path, int max_depth)\n> > +{\n> > +\treturn max_depth < 0 || count_chars(path, '/') <=\n> > max_depth; +}\n> > +\n> \n> If count_chars() is only used by accept_subdir() why not just manually\n> inline the loop in accept_subdir()? Plus, you rightly return early if\n> max_depth is -1, but what if max_depth is something small like 1? Then\n> you still count all the slashes when you could return upon seeing the\n> second slash.\n\nYep, that's true. My primary reason for writing two functions was to\nsplit logic in accept_subdir() from dull counting in count_chars().\nAlso, count_chars() is a generic function and may be used anywhere.\n\n> \n> >\n> > @@ -692,6 +724,8 @@ int cmd_grep(int argc, const char **argv, const\n> > char *prefix) OPT_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_INTEGER(0, \"max-depth\", &opt.max_depth,\n> > +\t\t\t\t\"descend at most levels\"),\n> \n> Please use OPTION_INTEGER here so you can use \"\" instead of\n> \"\". This will make it more consistent with the documentation.\n\nI can do that.\n\nThanks for your comments.\n\nMichał Kiedrowicz\n"},{"id":"118467","messageId":"7veis8myqr.fsf@alter.siamese.dyndns.org","threadId":"20187","inReplyTo":"4A66BB64.5070106@gmail.com","subject":"Re: [PATCH v3] grep: Add --max-depth option.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-22T17:08:44Z","receivedAt":"2009-07-22T17:08:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for a review. With the suggested changes the patch looks better.\n\nI had to wonder what would happen when you give an input like this,\nthough:\n\n $ git grep --max-depth=1 t Documentation/howto\n\nIf you look at the way the loop in pathspec_matches() is structured, it\nbecomes clear that the depth limit is applied independently to each of the\npathspecs given on the command line, and re-reading the description of the\noption in the documentation _with_ that knowledge it becomes obvious what\nshould happen, but I couldn't figure it out before looking at the code.\nPerhaps the documentation needs a bit more verbosity? I dunno.\n"}]}