Re: [PATCH v2 5/6] grep: enable recurse-submodules to work on objects

[Jonathan Tan]

On 10/31/2016 03:38 PM, Brandon Williams wrote:
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 17aa1ba..386a868 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,7 +26,7 @@ SYNOPSIS
           [--threads <num>]
           [-f <file>] [-e] <pattern>
           [--and|--or|--not|(|)|-e <pattern>...]
-          [--recurse-submodules]
+          [--recurse-submodules] [--parent-basename]

Maybe add something after --parent-basename, since it takes an argument 
(like --threads above).

@@ -91,7 +91,16 @@ OPTIONS

 --recurse-submodules::
        Recursively search in each submodule that has been initialized and
-       checked out in the repository.
+       checked out in the repository.  When used in combination with the
+       <tree> option the prefix of all submodule output will be the name of
+       the parent project's <tree> object.
+
+--parent-basename::

Same comment as above.

diff --git a/builtin/grep.c b/builtin/grep.c
index cf4f51e..2f10930 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -19,6 +19,7 @@
 #include "dir.h"
 #include "pathspec.h"
 #include "submodule.h"
+#include "submodule-config.h"

 static char const * const grep_usage[] = {
        N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
@@ -28,6 +29,7 @@ static char const * const grep_usage[] = {
 static const char *super_prefix;
 static int recurse_submodules;
 static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+static const char *parent_basename;

Can this be passed as an argument to the functions (grep_objects and 
grep_object, it seems) instead of having a file-visible variable?

@@ -671,12 +707,29 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
        enum interesting match = entry_not_interesting;
        struct name_entry entry;
        int old_baselen = base->len;
+       struct strbuf name = STRBUF_INIT;
+       int name_base_len = 0;
+       if (super_prefix) {
+               name_base_len = strlen(super_prefix);
+               strbuf_addstr(&name, super_prefix);

Better to invoke strbuf_addstr, and then set name_base_len from 
name.len. This makes it clear where strbuf_setlen (subsequently) resets 
the strbuf to, and is also a slight performance improvement.