Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes: >> + 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() >> + }; > > In the review club last week I had mentioned I thought OPT_CMDMODE > worked well with enums. I found some a reasonably nice example in > builtin/replace.c:cmd_replace(), although I have some Opinions about the > enum declaration placement there. Regardless, I think using an enum > instead of a single character would make this more readable - otherwise > I need to remember what 'w' means when I'm reasoning about how many args > to expect below. I am reasonably sure whoever did the above use of OPT_CMDMODE() feature mimicked an existing one that use it to make options with a single-letter shorthand mutually exclusive. If the options are with short-hand, you wouldn't be complaining that you do not know what 'w' stands for. When using it with options without short-hand, like this case, I'd agree it would make it easier to read to use symbolic constants of some kind.