On Tue, 20 Jun 2017 12:19:51 -0700 Brandon Williams <bmwill@xxxxxxxxxx> wrote: > -static void show_ce_entry(const struct index_state *istate, > - const char *tag, const struct cache_entry *ce) > +static void show_ce(struct repository *repo, struct dir_struct *dir, > + const struct cache_entry *ce, const char *fullname, > + const char *tag) This function is getting complicated - in particular, it's getting a "fullname" which is, in a sense, redundant with "repo" and "ce" (but that seems necessary for efficiency). Maybe add a comment describing the function? [snip] > @@ -651,8 +610,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) > if (require_work_tree && !is_inside_work_tree()) > setup_work_tree(); > > - if (recurse_submodules) > - compile_submodule_options(argv, &dir, show_tag); > + if (recurse_submodules) { > + repo_read_gitmodules(the_repository); > + } No need for braces.