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> >> >> 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. I confess I was not happy with the volume of the change either. I suppose I could break things down into renames vs parameter reorderings. Will update. >> 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. Will do (will reference the commit that introduced trailer.*.cmd also), thanks.
