Stefan Beller <sbel...@google.com> writes: > + for (; pp->count < pp->list.nr; pp->count++) { > + const struct submodule *sub = NULL; > + const struct cache_entry *ce = pp->list.entries[pp->count]; > + struct strbuf displaypath_sb = STRBUF_INIT; > + struct strbuf sb = STRBUF_INIT; > + const char *displaypath = NULL; > + char *url = NULL; > + int needs_cloning = 0; > + > + if (ce_stage(ce)) { > + if (pp->recursive_prefix) > + strbuf_addf(err, > + "Skipping unmerged submodule %s/%s\n", > + pp->recursive_prefix, ce->name);
The funny indentation of the string is a workaround for overly deep nesting, but is the overly deep nesting telling us that perhaps one iteration of this loop can be an invocation of a helper function, I wonder? > + else > + strbuf_addf(err, > + "Skipping unmerged submodule %s\n", > + ce->name); > + goto cleanup_and_continue; > + } > + > + sub = submodule_from_path(null_sha1, ce->name); > + > + if (pp->recursive_prefix) > + displaypath = relative_path(pp->recursive_prefix, > + ce->name, &displaypath_sb); > + else > + displaypath = ce->name; > + > + if ((pp->update && !strcmp(pp->update, "none")) || > + (!pp->update && sub->update == SM_UPDATE_NONE)) { This looks a bit strange. I wonder pp->update should also become enum for the same reason why sub->update has become enum. That way, we need to be worried about parsing these tokens in one place where a textual string "none" is translated to SM_UPDATE_NONE. If we started allowing "None" in the sub->update parse_config() in submodule-config.c, we would want that new parsing rule propagated to pp->update, right? > + strbuf_addf(err, "Skipping submodule '%s'\n", > + displaypath); > + goto cleanup_and_continue; > + } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html