On Tue, Aug 09 2022, Alban Gruin wrote:

+int merge_strategies_resolve(struct repository *r,
+			     struct commit_list *bases, const char *head_arg,
+			     struct commit_list *remote);

It would be very nice to have this prototype declared as a:

	typedef int (*merge_strategy_fn_t)(...);

Or whatever, so that when you later use this in 12/14. Then the end
state of this series could have this on top:
	
	diff --git a/merge-strategies.h b/merge-strategies.h
	index 8de2249ee6b..79b828105ba 100644
	--- a/merge-strategies.h
	+++ b/merge-strategies.h
	@@ -29,6 +29,9 @@ int merge_index_path(struct index_state *istate, int oneshot, int quiet,
	 int merge_all_index(struct index_state *istate, int oneshot, int quiet,
	 		    merge_fn fn, void *data);
	 
	+typedef int (*merge_strategy_fn_t)(struct repository *r,
	+			     struct commit_list *bases, const char *head_arg,
	+			     struct commit_list *remote);
	 int merge_strategies_resolve(struct repository *r,
	 			     struct commit_list *bases, const char *head_arg,
	 			     struct commit_list *remote);
	diff --git a/sequencer.c b/sequencer.c
	index 00a36205848..d5ef12dda27 100644
	--- a/sequencer.c
	+++ b/sequencer.c
	@@ -2309,6 +2309,7 @@ static int do_pick_commit(struct repository *r,
	 	} else {
	 		struct commit_list *common = NULL;
	 		struct commit_list *remotes = NULL;
	+		merge_strategy_fn_t fn = NULL;
	 
	 		res = write_message(msgbuf.buf, msgbuf.len,
	 				    git_path_merge_msg(r), 0);
	@@ -2316,12 +2317,14 @@ static int do_pick_commit(struct repository *r,
	 		commit_list_insert(base, &common);
	 		commit_list_insert(next, &remotes);
	 
	-		if (!strcmp(opts->strategy, "resolve")) {
	-			repo_read_index(r);
	-			res |= merge_strategies_resolve(r, common, oid_to_hex(&head), remotes);
	-		} else if (!strcmp(opts->strategy, "octopus")) {
	+		if (!strcmp(opts->strategy, "resolve"))
	+			fn = merge_strategies_resolve;
	+		else if (!strcmp(opts->strategy, "resolve"))
	+			fn = merge_strategies_octopus;
	+
	+		if (fn) {
	 			repo_read_index(r);
	-			res |= merge_strategies_octopus(r, common, oid_to_hex(&head), remotes);
	+			res |= fn(r, common, oid_to_hex(&head), remotes);
	 		} else {
	 			res |= try_merge_command(r, opts->strategy,
	 						 opts->xopts_nr, (const char **)opts->xopts,

We could replace that if/else if with a static array, and loop over it
to find the "fn" (if any), but I though it wasn't worth it just for
this.

This would also make my suggestion on top at
https://lore.kernel.org/git/220818.868rnlaa0h.gmgdl@evledraar.gmail.com/ (local)
nicer. I.e. we could just make that:

	if (git_env_bool("GIT_TEST_MERGE_COMMANDS", 0))
		fn = NULL;

And not need to add the "are we in the test mode" to the if/else if
branch for all of the internal strategies.