Thread (1 message) 1 message, 1 author, 2022-02-08

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help