[PATCH 0/4] Allow custom merge strategies, take 4

On Tue, Jul 29, 2008 at 12:47:05PM -0700, Junio C Hamano [off-list ref] wrote:

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?

I like the idea. ;-)

Here is what I did:

- I changed only load_command_list() and list_commands().
  load_command_list() still supports filtering, but the result is loaded
  to a pair of "struct cmdnames", supplied by the caller.

- list_commands() works from this pair, additionally supporting the
  custom title.

- export add_cmdname(), exclude_cmds() and is_in_cmdlist() without any
  modification in help.h

The rest is done in builtin-merge.c, as you suggested.

The nice thing is that before this change builtin-merge accepted for
example '-s index' (because git-merge-index was a valid command), but
now this is no longer true.

Miklos Vajna (4):
  builtin-help: make some internal functions available to other
    builtins
  builtin-merge: allow using a custom strategy
  Add a new test for using a custom merge strategy
  Add a second testcase for handling invalid strategies in git-merge

 Makefile                |    1 +
 builtin-merge.c         |   42 +++++++++++++++++++++----
 help.c                  |   77 ++++++++++++++++++++++++-----------------------
 help.h                  |   23 ++++++++++++++
 t/t7600-merge.sh        |    5 +++
 t/t7606-merge-custom.sh |   46 ++++++++++++++++++++++++++++
 6 files changed, 149 insertions(+), 45 deletions(-)
 create mode 100644 help.h
 create mode 100755 t/t7606-merge-custom.sh