Patrick Steinhardt <ps@xxxxxx> writes:
> On Fri, Mar 27, 2020 at 02:58:38PM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@xxxxxx> writes:
>>
>> > static const struct parse_cmd {
>> > const char *prefix;
>> > - const char *(*fn)(struct ref_transaction *, const char *, const char *);
>> > + void (*fn)(struct ref_transaction *, const char *, const char *);
>> > + unsigned extra_lines;
>>
>> Define and explain the meaning of extra_lines in a comment. Without
>> it, the most natural way to infer what the variable means by readers
>> would be that "update" command sometimes receives up to two more
>> lines but it also is perfectly normal if there is no extra line.
>>
>> But I am not sure if that is what is going on.
>>
>> "update" _always_ needs exactly two more input "lines" when the
>> input is NUL delimited, perhaps, and it is an _error_ if there are
>> not these two lines, no?
>
> That's extactly right. I pondered on a good name, but I wasn't yet able
> to come up with any one that brings across its meaning. I originally had
> `extra_args` but changed it after some internal discussion with Chris.
Wouldn't it make more sense to store the number of args expected
here, not "extra"? The one that does not take any then can say "I
am subcommand X and I do not want a SP after my name".
The "sometimes after the command there is line.termination, and
sometimes there is NUL, and yet some other times there is SP", which
is a sloppy way to check malformed input (i.e. you do not want SP
for a command that does not take parameters), that you have after
the loop can then go, if you did the parsing that way.
