On Wed, Feb 02 2022, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@xxxxxxxxx> > > Let merge-tree accept a `--write-tree` parameter for choosing real > merges instead of trivial merges, and accept an optional > `--trivial-merge` option to get the traditional behavior. Note that > these accept different numbers of arguments, though, so these names > need not actually be used. Maybe that ship has sailed, but just my 0.02: I thought this whole thing was much less confusing with your initial merge-tree-ort proposal at https://lore.kernel.org/git/CABPp-BEeBpJoU4yXdfA6vRAYVAUbd2gRhEV6j4VEqoqcu=FGSw@xxxxxxxxxxxxxx/; I.e. the end-state of merge-tree.c is that you end up reading largely unrelated code (various static functions only used by one side or another). But maybe that's all water under the bridge etc, however... > int cmd_merge_tree(int argc, const char **argv, const char *prefix) > { > - if (argc != 4) > - usage(merge_tree_usage); > - return trivial_merge(argc, argv); > + struct merge_tree_options o = { 0 }; > + int expected_remaining_argc; > + > + const char * const merge_tree_usage[] = { > + N_("git merge-tree [--write-tree] <branch1> <branch2>"), > + N_("git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2>"), > + NULL > + }; > + struct option mt_options[] = { > + OPT_CMDMODE(0, "write-tree", &o.mode, > + N_("do a real merge instead of a trivial merge"), > + 'w'), > + OPT_CMDMODE(0, "trivial-merge", &o.mode, > + N_("do a trivial merge only"), 't'), > + OPT_END() > + }; > + > + /* Parse arguments */ > + argc = parse_options(argc, argv, prefix, mt_options, > + merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION); > + if (o.mode) { > + expected_remaining_argc = (o.mode == 'w' ? 2 : 3); > + if (argc != expected_remaining_argc) > + usage_with_options(merge_tree_usage, mt_options); > + } else { > + if (argc < 2 || argc > 3) > + usage_with_options(merge_tree_usage, mt_options); > + o.mode = (argc == 2 ? 'w' : 't'); > + } Do we really need to make this interface more special-casey by auto-guessing based on argc what argument you want? I.e. instead of usage like: N_("git merge-tree [--write-tree] <branch1> <branch2>"), N_("git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2>"), Wouldn't it be simpler to just have the equivalent of: # old git merge-tree ... # new git merge-tree --new-thing ... And not have to look at ... to figure out if we're dispatching to the new or old thing.
