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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 21, 2023, 21:31 UTC
Message-ID: <xmqq4jreemdn.fsf@gitster.g>
In-Reply-To: <20230220140046.16986-1-tenglong.tl@alibaba-inc.com>

Teng Long <dyroneteng@gmail.com> writes:

Show 16 quoted lines

>> > 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?