On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Linus Arver <linusa@xxxxxxxxxx> > > The term "arg_item" is ambiguous because we use it to hold data for > > (1) trailers specified as command line arguments (in > builtin/interpret-trailers.c), and > > (2) trailers specified in configuration, > > and these are both used to ultimately insert new trailers (based > on the contents of arg_item, acting as a kind of template) into some > other set of existing trailers (such as those found in a trailer block > inside a log message) that have already been parsed. > > Rename "arg_item" to "trailer_template". This necessitates further > renames to make the functions that act on these templates match the data > structures (parameters) they act on: > > - [*] add_arg_to_input_list() to apply_template_to_trailers() > - [*] apply_arg_if_exists() to maybe_add_if_exists() > - [*] apply_arg_if_missing() to maybe_add_if_missing() > - apply_command() to run_command_from_template() > - [*] apply_item_command() to populate_template_value() > - free_arg_item() to free_template() (non-API function) > - free_new_trailers() to free_trailer_templates() (API function) > - get_conf_item() to get_or_add_template_by() > - option_parse_trailer() to option_parse_trailer_template() > - parse_trailers_from_config() to parse_trailer_templates_from_config() > - [*] process_trailers_lists() to apply_trailer_templates() > - token_from_item() to token_from_template() > - token_matches_item to token_matches_template > - [*] trailer_add_arg_item() to add_trailer_template() > - trailer_from_arg() to trailer_from() > - [*] check_if_different() (reorder parameters only) > - [*] find_same_and_apply_arg() (reorder parameters only) > > Reorder parameters to prefer input parameters toward the beginning and > out parameters at the end; these functions have been marked with an > asterisk ([*]). That's a lot of changes in a single patch. > This removes the "arg" terminology (standing for "CLI arguments") from > the trailer implementation, which makes sense because trailers > themselves have nothing to do with CLI argument handling. > > Also note that these renames expose the previously liberal use of > "trailer" to mean both trailers we read from the input text (trailer > block) and trailer templates that are defined as CLI args or > configurations. Some functions implied a single action when they could > do two different things, so introduce words like "maybe" and "or" to > make their behavior more explicit. > > In summary this patch renames and reorders parameters for readability, > without any behavioral change. We don't rename > find_same_and_apply_arg(), because it will be refactored soon. > > For parse_trailers_from_config() (renamed to > parse_trailer_templates_from_config()), add a NEEDSWORK discussion about > how the deprecated trailer.*.command configuration option is oddly more > featureful than trailer.*.cmd (if we were to remove support for > trailer.*.command, users would not be able to replicate the behavior > with trailer.*.cmd and would lose out on functionality). This change could be in a separate patch. Also there were discussions when trailer.*.command was deprecated and trailer.*.cmd introduced. I think it might be useful to talk about them in the commit message of the separate patch introducing the NEEDSWORK.
