Miklos Vajna <vmiklos@xxxxxxxxxxxxxx> writes:
> Allow using a custom strategy, as long as it's named git-merge-foo. The
> error handling is now done using is_git_command(). The list of available
> strategies is now shown by list_commands().
>
> If an invalid strategy is supplied, like -s foobar, then git-merge would
> list all git-merge-* commands. This is not perfect, since for example
> git-merge-index is not a valid strategy.
> ...
> + if (!is_git_command(name, "git-merge-")) {
> + struct cmdnames not_strategies;
> +
> + memset(¬_strategies, 0, sizeof(struct cmdnames));
> + for (i = 0; i < main_cmds.cnt; i++) {
> + int j, found = 0;
> + struct cmdname *ent = main_cmds.names[i];
> + for (j = 0; j < ARRAY_SIZE(all_strategy); j++)
> + if (!strncmp(ent->name, all_strategy[j].name, ent->len)
> + && !all_strategy[j].name[ent->len])
> + found = 1;
> + if (!found)
> + add_cmdname(¬_strategies, ent->name, ent->len);
> + }
This feels overly wasteful. Granted, this is not a performance critical
codepath, but you list all commands that begin with "git-merge-" in
is_git_command(), only to discard it and then iterate over 140+ main_cmds
list only to cull the ones whose name do not appear in the strategies
list.
Perhaps this shows that changing the function is_git_command() is a wrong
approach (for one thing, with the custom prefix, it is not about "Is it a
git command" anymore). Wouldn't it be easier to read if you did this part
like this instead?
* make load_command_list capable of loading into a "struct cmdnames" (or
pair if you want) supplied by the caller;
* use it to grab all commands whose name begin with "git-merge-" here;
* Check if name appears in that list; if it doesn't, you already have the
list of commands that could be merge strategy for error reporting.
If you also update list_commands() not to run load_command_list() itself
but take caller-supplied list of commands, then the API would become much
cleaner. The caller would not be limited to "filter with prefix" anymore.
Hmm?
--
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
