Re: [PATCH v3 03/15] merge-tree: add option parsing and initial shell for real merge function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux