Glen Choo <chooglen@xxxxxxxxxx> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> Previously, process_command_line_args did two things:
>>
>> (1) parse trailers from the configuration, and
>> (2) parse trailers defined on the command line.
>
> It parses trailers from two places, but it still only does "one thing",
> in that it only parses trailers.
More precisely, it parses trailers from the command line by first
parsing trailers from the configuration. In other words, parsing
trailers from the configuration (independent of the input string!) is a
required dependency for parsing trailers coming from the command line.
If we take a step back, we need to do 3 things:
(1) parse trailers from the configuration
(2) parse trailers from the command line
(3) parse trailers from the input
I think these three should be separated into separate functions. I think
no one wants to combine all three into one function. And I can't think
of a good enough reason to combine (1) and (2) together either. Hence
this patch.
> I find this equally readable as the preimage, which IMO is adequately
> scoped and commented.
Aside: is "preimage" the status quo before applying the patch?
>> @@ -1070,8 +1075,11 @@ void process_trailers(const char *file,
>>
>>
>> if (!opts->only_input) {
>> + LIST_HEAD(config_head);
>> LIST_HEAD(arg_head);
>> - process_command_line_args(&arg_head, new_trailer_head);
>> + parse_trailers_from_config(&config_head);
>> + parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
>> + list_splice(&config_head, &arg_head);
>> process_trailers_lists(&head, &arg_head);
>> }
>
> But now, we have to remember to call two functions instead of just one.
But only inside interpret-trailers.c, right?
