Subject: Re: [PATCH v5 3/3] notes.c: introduce "--separator" option

From: Junio C Hamano <[email-masked]>
Date: Tue, 21 Feb 2023 13:31:32 -0800

Teng Long <[email-masked]> writes:

>> > In addition, if a user specifies multple "-m" with "--separator", the
>> > separator should be inserted between the messages too, so we use
>> > OPT_STRING_LIST instead of OPT_CALLBACK_F to parse "-m" option, make
>> > sure the option value of "--separator" been parsed already when we need
>> > it.
>>
>> This is hard to grok.  Is it an instruction to whoever is
>> implementing this new feature, or is it an instruct to end-users
>> telling that they need to give --separator before they start giving
>> -m <msg>, -F <file>, -c <object>, etc.?
>
> No, it's not the order of the user give, but the backend we deal.
>
> We use "parse_msg_arg" as a callback when parsing "-m " by OPT_CALLBACK_F,
> so if we have to read the separator before we parse it, so we could insert
> it correctly between the messages, So I use OPT_STRING_LIST instead.

That is an implementation detail of how you chose to implement the
feature, and not an inherent limitation, is it?  It makes a lame
excuse to give users a hard-to-use UI.

For example, we could parse all the command line parameters without
making any action other than recording the strings given to -m and
contents of files given via -F in the order they appeared on the
command line, all in a single string list, while remembering the
last value of --separator you got, and then at the end concatenate
these strings using the final value of the separator, no?