Re: [PATCH v2 5/6] grep: enable recurse-submodules to work on <tree> objects
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Nov 11, 2016, 23:09 UTC
- Message-ID
- <36dd3a9b-39ae-4c5f-90d6-589820501ed4@google.com>
- In-Reply-To
- <1477953496-103596-6-git-send-email-bmwill@google.com>
On 10/31/2016 03:38 PM, Brandon Williams wrote:
Show 10 quoted lines
> 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).
Show 10 quoted lines
> @@ -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.
Show 17 quoted lines
> 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?
Show 9 quoted lines
> @@ -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.