Matheus Tavares <matheus.bernardino@xxxxxx> writes:
> @@ -475,7 +475,7 @@ static int grep_submodule(struct grep_opt *opt,
> strbuf_release(&base);
> free(data);
> } else {
> - hit = grep_cache(&subopt, pathspec, 1);
> + hit = grep_cache(&subopt, pathspec, cached);
> }
Interesting. It appears to me that this has always searched in
submodule index and never working tree. I am not sure if there was
any specific reason to avoid looking into the working tree files.
Passing the 'cached' bit down from grep_cache() does look like a
sensible way to go.
> @@ -523,7 +523,8 @@ static int grep_cache(struct grep_opt *opt,
> }
> } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
> submodule_path_match(repo->index, pathspec, name.buf, NULL)) {
> - hit |= grep_submodule(opt, pathspec, NULL, ce->name, ce->name);
> + hit |= grep_submodule(opt, pathspec, NULL, ce->name,
> + ce->name, cached);
> } else {
> continue;
> }
> @@ -598,7 +599,8 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> free(data);
> } else if (recurse_submodules && S_ISGITLINK(entry.mode)) {
> hit |= grep_submodule(opt, pathspec, &entry.oid,
> - base->buf, base->buf + tn_len);
> + base->buf, base->buf + tn_len,
> + 1); /* ignored */
The trailing comment is misleading. In the context of reviewing
this patch, we can probably tell it applies only to that "1", but
if you read only the postimage, the "ignored" comment looks as if
the call itself is somehow ignored by somebody unspecified. It is
not clear at all that it is only about the final parameter.
If you must...
hit |= grep_submodule(opt, pathspec, &entry.oid,
base->buf, base->buf + tn_len,
1 /* ignored */);
... is a reasonable way to write it.
> diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
> index a11366b4ce..946f91fa57 100755
> --- a/t/t7814-grep-recurse-submodules.sh
> +++ b/t/t7814-grep-recurse-submodules.sh
> @@ -408,4 +408,25 @@ test_expect_success 'grep --recurse-submodules with submodules without .gitmodul
> test_cmp expect actual
> '
>
> +reset_and_clean () {
> + git reset --hard &&
> + git clean -fd &&
> + git submodule foreach --recursive 'git reset --hard' &&
> + git submodule foreach --recursive 'git clean -fd'
Do we need two separate foreach, instread of a single one that does
both reset and clean (i.e. "git reset --hard && git clean -f -d")?
