Stefan Beller <sbeller@xxxxxxxxxx> writes:
> +static const char * const git_submodule_helper_usage[] = {
> + N_("git submodule--helper --module_list [<path>...]"),
Yuck. Please do not force --multi_word_opt upon us, which is simply
too ugly to live around here. --module-list is perhaps OK, but
because submodule--helper would not have an default action, I'd
prefer to make these just "command words", i.e.
$ git submodule--helper module_list
> +int module_list(int argc, const char **argv, const char *prefix)
> +{
> + int i;
> + static struct pathspec pathspec;
> + const struct cache_entry **ce_entries = NULL;
> + int alloc = 0, used = 0;
> + char *ps_matched = NULL;
> + char *max_prefix;
> + int max_prefix_len;
> + struct string_list already_printed = STRING_LIST_INIT_NODUP;
> +
> + parse_pathspec(&pathspec, 0,
> + PATHSPEC_PREFER_FULL,
> + prefix, argv);
> +
> + /* Find common prefix for all pathspec's */
> + max_prefix = common_prefix(&pathspec);
> + max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
> +
> + if (pathspec.nr)
> + ps_matched = xcalloc(1, pathspec.nr);
Up to this point it interprets its input, and ...
> + if (read_cache() < 0)
> + die("index file corrupt");
> +
> + for (i = 0; i < active_nr; i++) {
> + const struct cache_entry *ce = active_cache[i];
> +
> + if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
> + max_prefix_len, ps_matched,
> + S_ISGITLINK(ce->ce_mode) | S_ISDIR(ce->ce_mode)))
> + continue;
> +
> + if (S_ISGITLINK(ce->ce_mode)) {
> + ALLOC_GROW(ce_entries, used + 1, alloc);
> + ce_entries[used++] = ce;
> + }
> + }
> +
> + if (ps_matched && report_path_error(ps_matched, &pathspec, prefix)) {
> + printf("#unmatched\n");
> + return 1;
> + }
... does the computation, with diagnosis.
And then it does the I/O with formatting.
> +
> + for (i = 0; i < used; i++) {
> + const struct cache_entry *ce = ce_entries[i];
...
> + return 0;
> +}
When you have the implementation of "foreach-parallel" to move the
most expensive part of "submodule update" of a tree with 500
submodules, you would want to receive more or less the same "args"
as this thing takes and pass the ce_entries[] list to the "spawn and
run the user script in them in parallel" engine.
So I think it makes more sense to split this function into two (or
three). One that reads from (argc, argv) and allocates and fills
ce_entries[] can become a helper that you can reuse later.
'int module_list()' (shouldn't it be static?), can make a call to
that helper at the begining of it, and the remainder of the function
would do the textual I/O.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html