Christian Couder <christian.couder@xxxxxxxxx> writes:
> On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
>>
>> From: Linus Arver <linusa@xxxxxxxxxx>
>>
>> Move the logic of parse_trailer_from_command_line_arg() into
>> option_parse_trailer(), because that is the only caller and there's no
>> benefit in keeping these two separate.
>
> Well one benefit could be that 2 small functions might be easier to
> understand than a big one.
True.
> So perhaps
> parse_trailer_from_command_line_arg() could just have been made
> static?
In this case I don't think keeping these two functions separate would
make sense because parse_trailer_from_command_line_arg() is much more
heavyweight than option_parse_trailer() (one is just a thin wrapper
around the other). And I didn't like the thought of having 2 function
names that look very different:
parse_trailer_from_command_line_arg()
option_parse_trailer()
be so closely related in behavior.
And I already have some more patches (not in this series) that refactors
this area a bit also, so I wanted to wait until the dust settled down a
bit before deciding (esp. when unit tests are added) whether keep this
function separate. It's not clear to me yet whether we do want to add
unit tests for parse_trailer_from_command_line_arg() (if we do end up
"resurrecting it" in this patch or later), so IDK. The main reason is
because I think the first set of unit tests should be for the exposed
functions in <trailer.h>, not so much the helper functions that only the
builtin uses.
