Re: [PATCH v3 03/15] merge-tree: add option parsing and initial shell for real merge function
From: Junio C Hamano <hidden>
Date: 2022-02-08 01:07:44
Emily Shaffer [off-list ref] writes:
quoted
+ 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.