Re: [PATCH v5 5/6] grep: enable recurse-submodules to work on <tree> objects

[Junio C Hamano <gitster@pobox.com>]

Brandon Williams <bmwill@google.com> writes:

> diff --git a/tree-walk.c b/tree-walk.c
> index 828f435..ff77605 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -1004,6 +1004,19 @@ static enum interesting do_match(const struct name_entry *entry,
>  				 */
>  				if (ps->recursive && S_ISDIR(entry->mode))
>  					return entry_interesting;
> +
> +				/*
> +				 * When matching against submodules with
> +				 * wildcard characters, ensure that the entry
> +				 * at least matches up to the first wild
> +				 * character.  More accurate matching can then
> +				 * be performed in the submodule itself.
> +				 */
> +				if (ps->recursive && S_ISGITLINK(entry->mode) &&
> +				    !ps_strncmp(item, match + baselen,
> +						entry->path,
> +						item->nowildcard_len - baselen))
> +					return entry_interesting;
>  			}

This one (and the other hunk) feels more correct than the previous
round.  One thing to keep in mind however is that ps->recursive is
about "do we show a tree as a tree aka 040000, or do we descend into
it to show its contents?", not about "do we recurse into submodules?",
AFAICT.

So this change may have an impact on "git ls-tree -r" with pathspec;
I offhand do not know if that impact is undesirable or not.  A test
or two may be in order to illustrate what happens?  With a submodule
at "sub/module", running "git ls-tree -r HEAD -- sub/module/*" or
something like that, perhaps?