On 11/18, Junio C Hamano wrote: > Brandon Williams <bmwill@xxxxxxxxxx> writes: > > > @@ -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) { > > + strbuf_addstr(&name, super_prefix); > > + name_base_len = name.len; > > + } > > > > while (tree_entry(tree, &entry)) { > > int te_len = tree_entry_len(&entry); > > > > if (match != all_entries_interesting) { > > - match = tree_entry_interesting(&entry, base, tn_len, pathspec); > > + strbuf_setlen(&name, name_base_len); > > + strbuf_addstr(&name, base->buf + tn_len); > > + > > + if (recurse_submodules && S_ISGITLINK(entry.mode)) { > > + strbuf_addstr(&name, entry.path); > > + match = submodule_path_match(pathspec, name.buf, > > + NULL); > > The vocabulary from submodule_path_match() returns is the same as > that of do_match_pathspec() and match_pathspec_item() which is > MATCHED_{EXACTLY,FNMATCH,RECURSIVELY}, which is different from the > vocabulary of the variable "match" which is "enum interesting" that > is used by the tree-walk infrastructure. > > I doubt they are compatible to be usable like this. Am I missing > something? I think i initially must have thought it would work out, but looking back at this I can clearly see that they aren't 100% compatible... It slightly feels odd to me that we have so many different means for checking pathspecs, all of which pretty much duplicate some of the functionality of the other. Is there any reason there are these two different code paths? Do we want them to remain separate or have them be unified at some point? Also, in order to use the tree_entry_interesting code it looks like I'll either have to pipe through a flag saying 'yes i want to match against submodules' like I did for the other pathspec codepath. Either that or add functionality to perform wildmatching against partial matches (ie directories and submodules) since currently the tree_entry_interesting code path just punts and says 'well say it matches for now and check again later' whenever it runs into a directory (I can't really make it do that for submodules without a flag of somesort as tests could break). Or maybe both? -- Brandon Williams