Brandon Williams <bmw...@google.com> writes: > static void show_ce_entry(const char *tag, const struct cache_entry *ce) > { > + struct strbuf name = STRBUF_INIT; > int len = max_prefix_len; > + if (submodule_prefix) > + strbuf_addstr(&name, submodule_prefix); > + strbuf_addstr(&name, ce->name);
Continuing with the previous review, which concentrated on what happens in the superproject; let's see what happens in the recursive invocation in a submodule. So a recursively spawned "ls-files --submodule-prefix=sub/" finds a path in the index PATH and forms "sub/PATH" in "name". From here on, where we used to match pathspec against ce->name, we would be matching it against name->buf. > + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && > + submodule_path_match(&pathspec, name.buf, ps_matched)) { > show_gitlink(ce); This is primarily what happens in the superproject to decide if the submodule is worth showing. When we are in a submodule, we can descend into subsubmodule (if our ls-files run in the superproject passed --recurse-submodule down) from here. > + } else if (match_pathspec(&pathspec, name.buf, name.len, > + len, ps_matched, > + S_ISDIR(ce->ce_mode) || > + S_ISGITLINK(ce->ce_mode))) { This is interesting bit to see what happens in the recursive invocation. It uses the usual match_pathspec(), as we want to be precise and correct, iow, we do not want to use DO_MATCH_SUBMODULE, aka "it might be worth descending into submodule". > + if (tag && *tag && show_valid_bit && > + (ce->ce_flags & CE_VALID)) { > +... > + } > + write_eolinfo(ce, ce->name); > + write_name(ce->name); The prefixing is taken care of by write_name(), so it is correct to use ce->name here. > ... > + strbuf_release(&name); > } OK, everything I saw so far for the recursive invocation here makes sense. Thanks.