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@xxxxxxxxxxxxxxxxxxx/
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.
