wishlist: git grep -r

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.

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 <[email protected]>

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---

Quoted 9 lines · Christoph Berg

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(+)

Documentation/git-grep.txt +6 -0

index a3049af1a3..a1aea8be4e 100644
@@ -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.

builtin/grep.c +3 -0

index 601f801158..02d4384225 100644
@@ -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,

t/t7810-grep.sh +8 -1

index be5c1bd553..c48d1fa34b 100755
@@ -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

Quoted 3 lines · Æ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.

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)

Quoted 10 lines · Duy Nguyen

Quoted 3 lines · Æ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.

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.

builtin/grep.c +4 -2

index 02d4384225..2e048d9b49 100644
@@ -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);

t/t7810-grep.sh +4 -0

index c48d1fa34b..ad25cd50f5 100755
@@ -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

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 <[email protected]>

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---

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

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 <[email protected]>

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---

Quoted 9 lines · Christoph Berg

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(+)

Documentation/git-grep.txt +0 -0

index a3049af1a3..a1aea8be4e 100644

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)

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

builtin/grep.c +0 -0

index 601f801158..f6e127f0bc 100644

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)

Quoted 16 lines · René Scharfe

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

builtin/grep.c +0 -0

index 601f801158..f6e127f0bc 100644

builtin/grep.c int cmd_grep(int argc, const char **argv, const char *prefix) · line 815

Quoted 5 lines

builtin/grep.c +2 -0

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

Quoted 4 lines · Stefan Beller

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

Junio C Hamano wrote 2018-10-05 1:19 (-0700):

Quoted 7 lines

Quoted 4 lines · Stefan Beller

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)

Quoted 7 lines

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.