wishlist: git grep -r

wishlist: git grep -r

[Christoph Berg]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

I often use "grep -r $pattern" to recursively grep a source tree. If
that takes too long, I hit ^C and tag "git" in front of the command
line and re-run it. git then complains "error: unknown switch `r'"
because "git grep" is naturally recursive.

Could we have "git grep -r" accept the argument for compatibility?
Other important grep switches like "-i" are compatible, adding -r
would improve usability.

Thanks,
Christoph

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] grep: provide a noop --recursive option

[Ævar Arnfjörð Bjarmason]

This --recursive (-r) option does nothing, and is purely here to
appease people who have "grep -r ..." burned into their muscle memory.

Requested-by: Christoph Berg <myon@debian.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Sat, Sep 29, 2018 at 4:10 PM Christoph Berg <myon@debian.org> wrote:
>
> I often use "grep -r $pattern" to recursively grep a source tree. If
> that takes too long, I hit ^C and tag "git" in front of the command
> line and re-run it. git then complains "error: unknown switch `r'"
> because "git grep" is naturally recursive.
>
> Could we have "git grep -r" accept the argument for compatibility?
> Other important grep switches like "-i" are compatible, adding -r
> would improve usability.

I don't have an opinion on this either way, it doesn't scratch my
itch, but hey, why not. Here's a patch to implement it.

 Documentation/git-grep.txt | 6 ++++++
 builtin/grep.c             | 3 +++
 t/t7810-grep.sh            | 8 ++++++++
 3 files changed, 17 insertions(+)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index a3049af1a3..a1aea8be4e 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -290,6 +290,12 @@ providing this option will cause it to die.
 	Do not output matched lines; instead, exit with status 0 when
 	there is a match and with non-zero status when there isn't.
 
+-r::
+--recursive::
+	This option does nothing. git-grep is always recursive. This
+	noop option is provided for compatibility with the muscle
+	memory of people used to grep(1).
+
 <tree>...::
 	Instead of searching tracked files in the working tree, search
 	blobs in the given trees.
diff --git a/builtin/grep.c b/builtin/grep.c
index 601f801158..02d4384225 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -785,6 +785,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int use_index = 1;
 	int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
 	int allow_revs;
+	int unused_recursive; /* this is never used */
 
 	struct option options[] = {
 		OPT_BOOL(0, "cached", &cached,
@@ -802,6 +803,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			N_("show non-matching lines")),
 		OPT_BOOL('i', "ignore-case", &opt.ignore_case,
 			N_("case insensitive matching")),
+		OPT_BOOL('r', "recursive", &unused_recursive,
+			N_("does nothing, git-grep is always recursive, for grep(1) muscle memory compatibility")),
 		OPT_BOOL('w', "word-regexp", &opt.word_regexp,
 			N_("match patterns only at word boundaries")),
 		OPT_SET_INT('a', "text", &opt.binary,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index be5c1bd553..c48d1fa34b 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -469,6 +469,14 @@ do
 		git grep --count -h -e b $H -- ab >actual &&
 		test_cmp expected actual
 	'
+
+	for flag in '' ' -r' ' --recursive'
+	do
+		test_expect_success "grep $flag . (testing that --recursive is a noop)" '
+			git grep$flag . >actual &&
+			test_line_count = 43 actual
+	'
+	done
 done
 
 cat >expected <<EOF
-- 
2.19.0.605.g01d371f741

Re: [PATCH] grep: provide a noop --recursive option

[Duy Nguyen]

On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> This --recursive (-r) option does nothing, and is purely here to
> appease people who have "grep -r ..." burned into their muscle memory.

GNU grep -r recurses infinitely but Git grep also has --max-depth. How
do these interact? My knee-jerk reaction is -r equals --max-depth=-1
(i.e. overriding previous --mex-depth options on command line, or from
alias)

> Requested-by: Christoph Berg <myon@debian.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Sat, Sep 29, 2018 at 4:10 PM Christoph Berg <myon@debian.org> wrote:
> >
> > I often use "grep -r $pattern" to recursively grep a source tree. If
> > that takes too long, I hit ^C and tag "git" in front of the command
> > line and re-run it. git then complains "error: unknown switch `r'"
> > because "git grep" is naturally recursive.
> >
> > Could we have "git grep -r" accept the argument for compatibility?
> > Other important grep switches like "-i" are compatible, adding -r
> > would improve usability.
>
> I don't have an opinion on this either way, it doesn't scratch my
> itch, but hey, why not. Here's a patch to implement it.
>
>  Documentation/git-grep.txt | 6 ++++++
>  builtin/grep.c             | 3 +++
>  t/t7810-grep.sh            | 8 ++++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index a3049af1a3..a1aea8be4e 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -290,6 +290,12 @@ providing this option will cause it to die.
>         Do not output matched lines; instead, exit with status 0 when
>         there is a match and with non-zero status when there isn't.
>
> +-r::
> +--recursive::
> +       This option does nothing. git-grep is always recursive. This
> +       noop option is provided for compatibility with the muscle
> +       memory of people used to grep(1).
> +
>  <tree>...::
>         Instead of searching tracked files in the working tree, search
>         blobs in the given trees.
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 601f801158..02d4384225 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -785,6 +785,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>         int use_index = 1;
>         int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
>         int allow_revs;
> +       int unused_recursive; /* this is never used */
>
>         struct option options[] = {
>                 OPT_BOOL(0, "cached", &cached,
> @@ -802,6 +803,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>                         N_("show non-matching lines")),
>                 OPT_BOOL('i', "ignore-case", &opt.ignore_case,
>                         N_("case insensitive matching")),
> +               OPT_BOOL('r', "recursive", &unused_recursive,
> +                       N_("does nothing, git-grep is always recursive, for grep(1) muscle memory compatibility")),
>                 OPT_BOOL('w', "word-regexp", &opt.word_regexp,
>                         N_("match patterns only at word boundaries")),
>                 OPT_SET_INT('a', "text", &opt.binary,
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index be5c1bd553..c48d1fa34b 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -469,6 +469,14 @@ do
>                 git grep --count -h -e b $H -- ab >actual &&
>                 test_cmp expected actual
>         '
> +
> +       for flag in '' ' -r' ' --recursive'
> +       do
> +               test_expect_success "grep $flag . (testing that --recursive is a noop)" '
> +                       git grep$flag . >actual &&
> +                       test_line_count = 43 actual
> +       '
> +       done
>  done
>
>  cat >expected <<EOF
> --
> 2.19.0.605.g01d371f741
>


-- 
Duy

Re: [PATCH] grep: provide a noop --recursive option

[Ævar Arnfjörð Bjarmason]

On Sat, Sep 29 2018, Duy Nguyen wrote:

> On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> This --recursive (-r) option does nothing, and is purely here to
>> appease people who have "grep -r ..." burned into their muscle memory.
>
> GNU grep -r recurses infinitely but Git grep also has --max-depth. How
> do these interact? My knee-jerk reaction is -r equals --max-depth=-1
> (i.e. overriding previous --mex-depth options on command line, or from
> alias)

I didn't know about --max-depth, we could squash the following in to
deal with that, but that still leaves --max-depth=123 --no-recursive as
using depth 123, and we'd need different options parsing than OPT_BOOL
to deal with that.

diff --git a/builtin/grep.c b/builtin/grep.c
index 02d4384225..2e048d9b49 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -785,7 +785,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int use_index = 1;
 	int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
 	int allow_revs;
-	int unused_recursive; /* this is never used */
+	int recursive = 0;

 	struct option options[] = {
 		OPT_BOOL(0, "cached", &cached,
@@ -803,7 +803,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			N_("show non-matching lines")),
 		OPT_BOOL('i', "ignore-case", &opt.ignore_case,
 			N_("case insensitive matching")),
-		OPT_BOOL('r', "recursive", &unused_recursive,
+		OPT_BOOL('r', "recursive", &recursive,
 			N_("does nothing, git-grep is always recursive, for grep(1) muscle memory compatibility")),
 		OPT_BOOL('w', "word-regexp", &opt.word_regexp,
 			N_("match patterns only at word boundaries")),
@@ -926,6 +926,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	grep_commit_pattern_type(pattern_type_arg, &opt);

+	if (opt.max_depth != -1 && recursive)
+		die(_("The --max-depth and --recursive options are incompatible"));
 	if (use_index && !startup_info->have_repository) {
 		int fallback = 0;
 		git_config_get_bool("grep.fallbacktonoindex", &fallback);
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index c48d1fa34b..ad25cd50f5 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1697,4 +1697,8 @@ test_expect_success 'grep does not report i-t-a and assume unchanged with -L' '
 	test_cmp expected actual
 '

+test_expect_success 'grep --recursive is incompatible with --max-depth' '
+	test_must_fail git grep --recursive --max-depth=1
+'
+
 test_done

Re: [PATCH] grep: provide a noop --recursive option

[Duy Nguyen]

On Sat, Sep 29, 2018 at 5:25 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Sat, Sep 29 2018, Duy Nguyen wrote:
>
> > On Sat, Sep 29, 2018 at 4:58 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >>
> >> This --recursive (-r) option does nothing, and is purely here to
> >> appease people who have "grep -r ..." burned into their muscle memory.
> >
> > GNU grep -r recurses infinitely but Git grep also has --max-depth. How
> > do these interact? My knee-jerk reaction is -r equals --max-depth=-1
> > (i.e. overriding previous --mex-depth options on command line, or from
> > alias)
>
> I didn't know about --max-depth, we could squash the following in to
> deal with that, but that still leaves --max-depth=123 --no-recursive as
> using depth 123, and we'd need different options parsing than OPT_BOOL
> to deal with that.

I think if you initialize recursive variable below to -1, then you can
detect both --recursive (1) and --no-recursive (0).

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 02d4384225..2e048d9b49 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -785,7 +785,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>         int use_index = 1;
>         int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
>         int allow_revs;
> -       int unused_recursive; /* this is never used */
> +       int recursive = 0;
>
>         struct option options[] = {
>                 OPT_BOOL(0, "cached", &cached,
> @@ -803,7 +803,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>                         N_("show non-matching lines")),
>                 OPT_BOOL('i', "ignore-case", &opt.ignore_case,
>                         N_("case insensitive matching")),
> -               OPT_BOOL('r', "recursive", &unused_recursive,
> +               OPT_BOOL('r', "recursive", &recursive,
>                         N_("does nothing, git-grep is always recursive, for grep(1) muscle memory compatibility")),
>                 OPT_BOOL('w', "word-regexp", &opt.word_regexp,
>                         N_("match patterns only at word boundaries")),
> @@ -926,6 +926,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>                              PARSE_OPT_STOP_AT_NON_OPTION);
>         grep_commit_pattern_type(pattern_type_arg, &opt);
>
> +       if (opt.max_depth != -1 && recursive)
> +               die(_("The --max-depth and --recursive options are incompatible"));
>         if (use_index && !startup_info->have_repository) {
>                 int fallback = 0;
>                 git_config_get_bool("grep.fallbacktonoindex", &fallback);
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index c48d1fa34b..ad25cd50f5 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -1697,4 +1697,8 @@ test_expect_success 'grep does not report i-t-a and assume unchanged with -L' '
>         test_cmp expected actual
>  '
>
> +test_expect_success 'grep --recursive is incompatible with --max-depth' '
> +       test_must_fail git grep --recursive --max-depth=1
> +'
> +
>  test_done



-- 
Duy

Re: [PATCH] grep: provide a noop --recursive option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This --recursive (-r) option does nothing, and is purely here to
> appease people who have "grep -r ..." burned into their muscle memory.
>
> Requested-by: Christoph Berg <myon@debian.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

I personally am not all that sympathetic to the "'git grep' and
'grep' sound similar, so even though it won't do anything useful
just add a synonym to noop" line of reasoning.  That will lead to
sloppy noop, and will invite unbound amount of busywork to deal with
future complaints like "oh, but 'grep GNU COPYING' does not give
useless filename in front like the same command line with 'git'
prefixed; please fix 'git grep'" (which we'd have to say "no",
wasting our time).

I however do not mind if we added "--recursive" with matching
"--no-recursive", and

 - made "--recursive" the default (obviously)

 - made "--no-recursive" a synonym to setting the recursion limit
   to "never recurse"

 - and made "--recursive" a synonym to setting the recursion limit
   to "infinity".

That would be more work than this patch.  But if I see "--recursive"
advertised as a feature, and the command by default goes recursive,
I do expect to be able to tell it not to recurse.

I also expect folks who are used to "git grep --re<TAB>" to summon
the only option of the command that begins with that prefix to start
complaining that they now have to type "--recurs<TAB>" instead.  I
am not solving that with the above suggestion to improve the
suggested "noop".

Re: [PATCH] grep: provide a noop --recursive option

[Christoph Berg]

Re: Junio C Hamano 2018-09-29 <xmqq8t3k9qjs.fsf@gitster-ct.c.googlers.com>
> I also expect folks who are used to "git grep --re<TAB>" to summon
> the only option of the command that begins with that prefix to start
> complaining that they now have to type "--recurs<TAB>" instead.

Fwiw I was just asking about -r. There doesn't have to be a
corresponding long option if the short one is a no-op.

Christoph

Re: [PATCH] grep: provide a noop --recursive option

[René Scharfe]

Am 29.09.2018 um 19:11 schrieb Junio C Hamano:
> I however do not mind if we added "--recursive" with matching
> "--no-recursive", and
> 
>  - made "--recursive" the default (obviously)
> 
>  - made "--no-recursive" a synonym to setting the recursion limit
>    to "never recurse"
> 
>  - and made "--recursive" a synonym to setting the recursion limit
>    to "infinity".
> 
> That would be more work than this patch.  But if I see "--recursive"
> advertised as a feature, and the command by default goes recursive,
> I do expect to be able to tell it not to recurse.

-- >8 --
Subject: [PATCH] grep: add -r/--[no-]recursive

Recognize -r and --recursive as synonyms for --max-depth=-1 for
compatibility with GNU grep; it's still the default for git grep.

This also adds --no-recursive as synonym for --max-depth=0 for free,
which is welcome for completeness and consistency.

Fix the description for --max-depth, while we're at it -- negative
values other than -1 actually disable recursion, i.e. they are
equivalent to --max-depth=0.

Requested-by: Christoph Berg <myon@debian.org>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Initial-patch-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 Documentation/git-grep.txt | 11 +++++++++--
 builtin/grep.c             |  2 ++
 t/t7810-grep.sh            | 12 ++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index a3049af1a3..84fe236a8e 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -18,7 +18,7 @@ SYNOPSIS
 	   [(-O | --open-files-in-pager) [<pager>]]
 	   [-z | --null]
 	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
-	   [--max-depth <depth>]
+	   [--max-depth <depth>] [--[no-]recursive]
 	   [--color[=<when>] | --no-color]
 	   [--break] [--heading] [-p | --show-function]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
@@ -119,11 +119,18 @@ OPTIONS
 
 --max-depth <depth>::
 	For each <pathspec> given on command line, descend at most <depth>
-	levels of directories. A negative value means no limit.
+	levels of directories. A value of -1 means no limit.
 	This option is ignored if <pathspec> contains active wildcards.
 	In other words if "a*" matches a directory named "a*",
 	"*" is matched literally so --max-depth is still effective.
 
+-r::
+--recursive::
+	Same as `--max-depth=-1`; this is the default.
+
+--no-recursive::
+	Same as `--max-depth=0`.
+
 -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 601f801158..f6e127f0bc 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -811,6 +811,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			GREP_BINARY_NOMATCH),
 		OPT_BOOL(0, "textconv", &opt.allow_textconv,
 			 N_("process binary files with textconv filters")),
+		OPT_SET_INT('r', "recursive", &opt.max_depth,
+			    N_("search in subdirectories (default)"), -1),
 		{ OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
 			N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
 			NULL, 1 },
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index be5c1bd553..43aa4161cf 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -309,6 +309,8 @@ do
 			echo ${HC}v:1:vvv
 		} >expected &&
 		git grep --max-depth -1 -n -e vvv $H >actual &&
+		test_cmp expected actual &&
+		git grep --recursive -n -e vvv $H >actual &&
 		test_cmp expected actual
 	'
 
@@ -317,6 +319,8 @@ do
 			echo ${HC}v:1:vvv
 		} >expected &&
 		git grep --max-depth 0 -n -e vvv $H >actual &&
+		test_cmp expected actual &&
+		git grep --no-recursive -n -e vvv $H >actual &&
 		test_cmp expected actual
 	'
 
@@ -327,6 +331,8 @@ do
 			echo ${HC}v:1:vvv
 		} >expected &&
 		git grep --max-depth 0 -n -e vvv $H -- "*" >actual &&
+		test_cmp expected actual &&
+		git grep --no-recursive -n -e vvv $H -- "*" >actual &&
 		test_cmp expected actual
 	'
 
@@ -344,6 +350,8 @@ do
 			echo ${HC}t/v:1:vvv
 		} >expected &&
 		git grep --max-depth 0 -n -e vvv $H -- t >actual &&
+		test_cmp expected actual &&
+		git grep --no-recursive -n -e vvv $H -- t >actual &&
 		test_cmp expected actual
 	'
 
@@ -353,6 +361,8 @@ do
 			echo ${HC}v:1:vvv
 		} >expected &&
 		git grep --max-depth 0 -n -e vvv $H -- . t >actual &&
+		test_cmp expected actual &&
+		git grep --no-recursive -n -e vvv $H -- . t >actual &&
 		test_cmp expected actual
 	'
 
@@ -362,6 +372,8 @@ do
 			echo ${HC}v:1:vvv
 		} >expected &&
 		git grep --max-depth 0 -n -e vvv $H -- t . >actual &&
+		test_cmp expected actual &&
+		git grep --no-recursive -n -e vvv $H -- t . >actual &&
 		test_cmp expected actual
 	'
 	test_expect_success "grep $L with grep.extendedRegexp=false" '
-- 
2.19.0

Re: [PATCH] grep: provide a noop --recursive option

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

>
> Recognize -r and --recursive as synonyms for --max-depth=-1 for
> compatibility with GNU grep; it's still the default for git grep.
>
> This also adds --no-recursive as synonym for --max-depth=0 for free,
> which is welcome for completeness and consistency.
>
> Fix the description for --max-depth, while we're at it -- negative
> values other than -1 actually disable recursion, i.e. they are
> equivalent to --max-depth=0.
> ...
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 601f801158..f6e127f0bc 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -811,6 +811,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  			GREP_BINARY_NOMATCH),
>  		OPT_BOOL(0, "textconv", &opt.allow_textconv,
>  			 N_("process binary files with textconv filters")),
> +		OPT_SET_INT('r', "recursive", &opt.max_depth,
> +			    N_("search in subdirectories (default)"), -1),

Wow.  

I didn't think of this trick to let OPT_SET_INT() to grok --no-* and
set the variable to 0.  Being able to do this without a custom
callback is certainly very nice.

The patch looks good.

Thanks.

Re: [PATCH] grep: provide a noop --recursive option

[Ævar Arnfjörð Bjarmason]

On Fri, Oct 05 2018, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
>
>>
>> Recognize -r and --recursive as synonyms for --max-depth=-1 for
>> compatibility with GNU grep; it's still the default for git grep.
>>
>> This also adds --no-recursive as synonym for --max-depth=0 for free,
>> which is welcome for completeness and consistency.
>>
>> Fix the description for --max-depth, while we're at it -- negative
>> values other than -1 actually disable recursion, i.e. they are
>> equivalent to --max-depth=0.
>> ...
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 601f801158..f6e127f0bc 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -811,6 +811,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>>  			GREP_BINARY_NOMATCH),
>>  		OPT_BOOL(0, "textconv", &opt.allow_textconv,
>>  			 N_("process binary files with textconv filters")),
>> +		OPT_SET_INT('r', "recursive", &opt.max_depth,
>> +			    N_("search in subdirectories (default)"), -1),
>
> Wow.
>
> I didn't think of this trick to let OPT_SET_INT() to grok --no-* and
> set the variable to 0.  Being able to do this without a custom
> callback is certainly very nice.
>
> The patch looks good.

FWIW I'm not going to carry this series forward, I wrote it more as a
"here's how this could be done". But if Christoph / René wants to hack
on it...

Re: [PATCH] grep: provide a noop --recursive option

[Stefan Beller]

On Sat, Sep 29, 2018 at 7:55 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> This --recursive (-r) option does nothing, and is purely here to
> appease people who have "grep -r ..." burned into their muscle memory.
>
> Requested-by: Christoph Berg <myon@debian.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Sat, Sep 29, 2018 at 4:10 PM Christoph Berg <myon@debian.org> wrote:
> >
> > I often use "grep -r $pattern" to recursively grep a source tree. If
> > that takes too long, I hit ^C and tag "git" in front of the command
> > line and re-run it. git then complains "error: unknown switch `r'"
> > because "git grep" is naturally recursive.
> >
> > Could we have "git grep -r" accept the argument for compatibility?
> > Other important grep switches like "-i" are compatible, adding -r
> > would improve usability.
>
> I don't have an opinion on this either way, it doesn't scratch my
> itch, but hey, why not. Here's a patch to implement it.
>
>  Documentation/git-grep.txt | 6 ++++++
>  builtin/grep.c             | 3 +++
>  t/t7810-grep.sh            | 8 ++++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index a3049af1a3..a1aea8be4e 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -290,6 +290,12 @@ providing this option will cause it to die.
>         Do not output matched lines; instead, exit with status 0 when
>         there is a match and with non-zero status when there isn't.
>
> +-r::
> +--recursive::
> +       This option does nothing. git-grep is always recursive. This
> +       noop option is provided for compatibility with the muscle
> +       memory of people used to grep(1).

git-grep is always file/tree recursive, but there is --recurse-submodules
which is off by default. Instead of providing a short alias to a noop,
we could use -r for submodules. (And if you happen to have no
submodules, this is a noop for you)

Re: [PATCH] grep: provide a noop --recursive option

[Christoph Berg]

Re: Stefan Beller 2018-10-01 <CAGZ79kbw96x2Dow7d-sUfOHXiVN8j9KgqEObo+TrVd5zWKbaEA@mail.gmail.com>
> git-grep is always file/tree recursive, but there is --recurse-submodules
> which is off by default. Instead of providing a short alias to a noop,
> we could use -r for submodules. (And if you happen to have no
> submodules, this is a noop for you)

That would be fine for me.

Thanks,
Christoph

Re: [PATCH] grep: provide a noop --recursive option

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> git-grep is always file/tree recursive, but there is --recurse-submodules
> which is off by default. Instead of providing a short alias to a noop,
> we could use -r for submodules. (And if you happen to have no
> submodules, this is a noop for you)

I am not sure if it is an overall win for those who do have and use
submodules to easily be able to go recursive with a short-and-sweet
'r', or even they want to work inside one project at a time most of
the time.  If the latter, then using 'r' for recurse-submodules is
going to be a mistake (besides, other commands that have 'recursive'
typically use 'r' for its shorthand,and 'r' does not stand for
'recurse-submodules' for them).

Re: [PATCH] grep: provide a noop --recursive option

[Mischa POSLAWSKY]

Junio C Hamano wrote 2018-10-05 1:19 (-0700):
> Stefan Beller <sbeller@google.com> writes:
> 
> > git-grep is always file/tree recursive, but there is --recurse-submodules
> > which is off by default. Instead of providing a short alias to a noop,
> > we could use -r for submodules. (And if you happen to have no
> > submodules, this is a noop for you)
> 
> I am not sure if it is an overall win for those who do have and use
> submodules to easily be able to go recursive with a short-and-sweet
> 'r', or even they want to work inside one project at a time most of
> the time.  If the latter, then using 'r' for recurse-submodules is
> going to be a mistake (besides, other commands that have 'recursive'
> typically use 'r' for its shorthand,and 'r' does not stand for
> 'recurse-submodules' for them).

Personally I would welcome a shorthand for --recurse-submodules,
especially if --r^I no longer completes to this.

It is also closer to the behaviour provided by grep -r as that recurses
into submodules as well.

Regards,
-- 
Mischa

Re: [PATCH] grep: provide a noop --recursive option

[Stefan Beller]

On Fri, Oct 5, 2018 at 6:05 AM Mischa POSLAWSKY <git@shiar.nl> wrote:
>
> Junio C Hamano wrote 2018-10-05 1:19 (-0700):
> > Stefan Beller <sbeller@google.com> writes:
> >
> > > git-grep is always file/tree recursive, but there is --recurse-submodules
> > > which is off by default. Instead of providing a short alias to a noop,
> > > we could use -r for submodules. (And if you happen to have no
> > > submodules, this is a noop for you)
> >
> > I am not sure if it is an overall win for those who do have and use
> > submodules to easily be able to go recursive with a short-and-sweet
> > 'r', or even they want to work inside one project at a time most of
> > the time.  If the latter, then using 'r' for recurse-submodules is
> > going to be a mistake (besides, other commands that have 'recursive'
> > typically use 'r' for its shorthand,and 'r' does not stand for
> > 'recurse-submodules' for them).
>
> Personally I would welcome a shorthand for --recurse-submodules,
> especially if --r^I no longer completes to this.

The new switch differs by one dash, so I'd think the double dashed
version would still autocomplete.

Unrelated to this, but more to submodules:
There is submodule.recurse which you may want to set.
Would you be interested in a more specific config option there?
(i.e. grep.recurseSubmodules to only apply to grep recursing into
submodules, just like fetch.recurseSubmodules only applies to fetch)

> It is also closer to the behaviour provided by grep -r as that recurses
> into submodules as well.

That sort of makes for the grep case, but not for other commands.
See the related discussion at
https://public-inbox.org/git/20180907064026.GB172953@aiede.svl.corp.google.com/