git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 10/10] trailer: introduce "template" term for readability

From
LGLinus Arver via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 31, 2024, 01:22 UTC
Message-ID
<bcd3fc9660e08f5e8fea7c8c85bdd69aae3b1e93.1706664145.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1632.v3.git.1706664144.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
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 where appropriate; these functions have been marked with an asterisk ([*]).

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 unmask their behavior.

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. As an added benefit, we no longer use the term "arg" to mean "CLI arguments" in the trailer implementation, because trailers themselves should not be concerned about CLI arguments (this is the domain of the interpret-trailers builtin).

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 lose out on functionality).

Signed-off-by: Linus Arver <linusa@google.com>
---
 builtin/interpret-trailers.c |  48 +++---
 trailer.c                    | 292 +++++++++++++++++++----------------
 trailer.h                    |  11 +-
 3 files changed, 192 insertions(+), 159 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 9a902012912..aae3f3119df 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -47,10 +47,14 @@ static int option_parse_if_missing(const struct option *opt,
 
 static char *cl_separators;
 
-static int option_parse_trailer(const struct option *opt,
-				   const char *arg, int unset)
+/*
+ * Interpret "--trailer ..." as trailer templates (trailers we want to add into
+ * the input text).
+ */
+static int option_parse_trailer_template(const struct option *opt,
+					 const char *arg, int unset)
 {
-	struct list_head *trailers = opt->value;
+	struct list_head *templates = opt->value;
 	struct strbuf tok = STRBUF_INIT;
 	struct strbuf val = STRBUF_INIT;
 	const struct trailer_conf *conf;
@@ -58,7 +62,7 @@ static int option_parse_trailer(const struct option *opt,
 	ssize_t separator_pos;
 
 	if (unset) {
-		free_new_trailers(trailers);
+		free_trailer_templates(templates);
 		return 0;
 	}
 
@@ -75,10 +79,10 @@ static int option_parse_trailer(const struct option *opt,
 		 */
 		trailer_conf_set(where, if_exists, if_missing, conf_current);
 
-		trailer_add_arg_item(strbuf_detach(&tok, NULL),
+		add_trailer_template(strbuf_detach(&tok, NULL),
 				     strbuf_detach(&val, NULL),
 				     conf_current,
-				     trailers);
+				     templates);
 	} else {
 		struct strbuf sb = STRBUF_INIT;
 		strbuf_addstr(&sb, arg);
@@ -148,10 +152,10 @@ static void read_input_file(struct strbuf *sb, const char *file)
 }
 
 static void interpret_trailers(const struct process_trailer_options *opts,
-			       struct list_head *arg_trailers,
+			       struct list_head *templates,
 			       const char *file)
 {
-	LIST_HEAD(head);
+	LIST_HEAD(trailers_from_sb);
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf tb = STRBUF_INIT;
 	struct trailer_block *trailer_block;
@@ -162,7 +166,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	if (opts->in_place)
 		outfile = create_in_place_tempfile(file);
 
-	trailer_block = parse_trailers(opts, sb.buf, &head);
+	trailer_block = parse_trailers(opts, sb.buf, &trailers_from_sb);
 
 	/* Print the lines before the trailer block */
 	if (!opts->only_trailers)
@@ -173,15 +177,15 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 
 
 	if (!opts->only_input) {
-		process_trailers_lists(&head, arg_trailers);
+		apply_trailer_templates(templates, &trailers_from_sb);
 	}
 
 	/* Print trailer block. */
-	format_trailers(opts, &head, &tb);
+	format_trailers(opts, &trailers_from_sb, &tb);
 	fwrite(tb.buf, 1, tb.len, outfile);
 	strbuf_release(&tb);
 
-	free_trailers(&head);
+	free_trailers(&trailers_from_sb);
 
 	/* Print the lines after the trailer block as is */
 	if (!opts->only_trailers)
@@ -199,8 +203,8 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 {
 	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
-	LIST_HEAD(configured_trailers);
-	LIST_HEAD(arg_trailers);
+	LIST_HEAD(configured_templates);
+	LIST_HEAD(templates);
 
 	struct option options[] = {
 		OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")),
@@ -219,8 +223,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK_F(0, "parse", &opts, NULL, N_("alias for --only-trailers --only-input --unfold"),
 			PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse),
 		OPT_BOOL(0, "no-divider", &opts.no_divider, N_("do not treat \"---\" as the end of input")),
-		OPT_CALLBACK(0, "trailer", &arg_trailers, N_("trailer"),
-				N_("trailer(s) to add"), option_parse_trailer),
+		OPT_CALLBACK(0, "trailer", &templates, N_("trailer"),
+				N_("trailer(s) to add"), option_parse_trailer_template),
 		OPT_END()
 	};
 
@@ -228,7 +232,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 	trailer_config_init();
 
 	if (!opts.only_input) {
-		parse_trailers_from_config(&configured_trailers);
+		parse_trailer_templates_from_config(&configured_templates);
 	}
 
 	/*
@@ -242,25 +246,25 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 
 	free(cl_separators);
 
-	if (opts.only_input && !list_empty(&arg_trailers))
+	if (opts.only_input && !list_empty(&templates))
 		usage_msg_opt(
 			_("--trailer with --only-input does not make sense"),
 			git_interpret_trailers_usage,
 			options);
 
-	list_splice(&configured_trailers, &arg_trailers);
+	list_splice(&configured_templates, &templates);
 
 	if (argc) {
 		int i;
 		for (i = 0; i < argc; i++)
-			interpret_trailers(&opts, &arg_trailers, argv[i]);
+			interpret_trailers(&opts, &templates, argv[i]);
 	} else {
 		if (opts.in_place)
 			die(_("no input file given for in-place editing"));
-		interpret_trailers(&opts, &arg_trailers, NULL);
+		interpret_trailers(&opts, &templates, NULL);
 	}
 
-	free_new_trailers(&arg_trailers);
+	free_trailer_templates(&templates);
 
 	return 0;
 }
diff --git a/trailer.c b/trailer.c
index bf1d2eee550..a739e2ada86 100644
--- a/trailer.c
+++ b/trailer.c
@@ -55,14 +55,14 @@ struct trailer_item {
 	char *value;
 };
 
-struct arg_item {
+struct trailer_template {
 	struct list_head list;
 	char *token;
 	char *value;
 	struct trailer_conf conf;
 };
 
-static LIST_HEAD(conf_head);
+static LIST_HEAD(templates_from_conf);
 
 static char *separators = ":";
 
@@ -105,7 +105,7 @@ static size_t token_len_without_separator(const char *token, size_t len)
 	return len;
 }
 
-static int same_token(struct trailer_item *a, struct arg_item *b)
+static int same_token(struct trailer_item *a, struct trailer_template *b)
 {
 	size_t a_len, b_len, min_len;
 
@@ -119,12 +119,12 @@ static int same_token(struct trailer_item *a, struct arg_item *b)
 	return !strncasecmp(a->token, b->token, min_len);
 }
 
-static int same_value(struct trailer_item *a, struct arg_item *b)
+static int same_value(struct trailer_item *a, struct trailer_template *b)
 {
 	return !strcasecmp(a->value, b->value);
 }
 
-static int same_trailer(struct trailer_item *a, struct arg_item *b)
+static int same_trailer(struct trailer_item *a, struct trailer_template *b)
 {
 	return same_token(a, b) && same_value(a, b);
 }
@@ -151,15 +151,15 @@ static void free_trailer_item(struct trailer_item *item)
 	free(item);
 }
 
-static void free_arg_item(struct arg_item *item)
+static void free_template(struct trailer_template *template)
 {
-	free(item->conf.name);
-	free(item->conf.key);
-	free(item->conf.command);
-	free(item->conf.cmd);
-	free(item->token);
-	free(item->value);
-	free(item);
+	free(template->conf.name);
+	free(template->conf.key);
+	free(template->conf.command);
+	free(template->conf.cmd);
+	free(template->token);
+	free(template->value);
+	free(template);
 }
 
 static char last_non_space_char(const char *s)
@@ -171,36 +171,36 @@ static char last_non_space_char(const char *s)
 	return '\0';
 }
 
-static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
+static struct trailer_item *trailer_from(struct trailer_template *template)
 {
 	struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
-	new_item->token = arg_tok->token;
-	new_item->value = arg_tok->value;
-	arg_tok->token = arg_tok->value = NULL;
-	free_arg_item(arg_tok);
+	new_item->token = template->token;
+	new_item->value = template->value;
+	template->token = template->value = NULL;
+	free_template(template);
 	return new_item;
 }
 
-static void add_arg_to_input_list(struct trailer_item *on_tok,
-				  struct arg_item *arg_tok)
+static void apply_template_to_trailers(struct trailer_template *template,
+				       struct trailer_item *on_tok)
 {
-	int aoe = after_or_end(arg_tok->conf.where);
-	struct trailer_item *to_add = trailer_from_arg(arg_tok);
+	int aoe = after_or_end(template->conf.where);
+	struct trailer_item *to_add = trailer_from(template);
 	if (aoe)
 		list_add(&to_add->list, &on_tok->list);
 	else
 		list_add_tail(&to_add->list, &on_tok->list);
 }
 
-static int check_if_different(struct trailer_item *in_tok,
-			      struct arg_item *arg_tok,
-			      int check_all,
-			      struct list_head *head)
+static int check_if_different(struct trailer_template *template,
+			      struct trailer_item *in_tok,
+			      struct list_head *head,
+			      int check_all)
 {
-	enum trailer_where where = arg_tok->conf.where;
+	enum trailer_where where = template->conf.where;
 	struct list_head *next_head;
 	do {
-		if (same_trailer(in_tok, arg_tok))
+		if (same_trailer(in_tok, template))
 			return 0;
 		/*
 		 * if we want to add a trailer after another one,
@@ -215,7 +215,8 @@ static int check_if_different(struct trailer_item *in_tok,
 	return 1;
 }
 
-static char *apply_command(struct trailer_conf *conf, const char *arg)
+static char *run_command_from_template(struct trailer_conf *conf,
+				       const char *arg)
 {
 	struct strbuf cmd = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
@@ -250,133 +251,142 @@ static char *apply_command(struct trailer_conf *conf, const char *arg)
 	return result;
 }
 
-static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)
+/*
+ * Prepare the template by running the command (if any) requested by the
+ * template in order to populate the template's value field.
+ */
+static void populate_template_value(struct trailer_template *template,
+				    struct trailer_item *in_tok)
 {
-	if (arg_tok->conf.command || arg_tok->conf.cmd) {
+	if (template->conf.command || template->conf.cmd) {
+		/*
+		 * Determine argument to pass into the command.
+		 */
 		const char *arg;
-		if (arg_tok->value && arg_tok->value[0]) {
-			arg = arg_tok->value;
+		if (template->value && template->value[0]) {
+			arg = template->value;
 		} else {
 			if (in_tok && in_tok->value)
 				arg = xstrdup(in_tok->value);
 			else
 				arg = xstrdup("");
 		}
-		arg_tok->value = apply_command(&arg_tok->conf, arg);
+		template->value = run_command_from_template(&template->conf,
+							    arg);
 		free((char *)arg);
 	}
 }
 
-static void apply_arg_if_exists(struct trailer_item *in_tok,
-				struct arg_item *arg_tok,
+static void maybe_add_if_exists(struct trailer_template *template,
+				struct trailer_item *in_tok,
 				struct trailer_item *on_tok,
-				struct list_head *head)
+				struct list_head *trailers)
 {
-	switch (arg_tok->conf.if_exists) {
+	switch (template->conf.if_exists) {
 	case EXISTS_DO_NOTHING:
-		free_arg_item(arg_tok);
+		free_template(template);
 		break;
 	case EXISTS_REPLACE:
-		apply_item_command(in_tok, arg_tok);
-		add_arg_to_input_list(on_tok, arg_tok);
+		populate_template_value(template, in_tok);
+		apply_template_to_trailers(template, on_tok);
 		list_del(&in_tok->list);
 		free_trailer_item(in_tok);
 		break;
 	case EXISTS_ADD:
-		apply_item_command(in_tok, arg_tok);
-		add_arg_to_input_list(on_tok, arg_tok);
+		populate_template_value(template, in_tok);
+		apply_template_to_trailers(template, on_tok);
 		break;
 	case EXISTS_ADD_IF_DIFFERENT:
-		apply_item_command(in_tok, arg_tok);
-		if (check_if_different(in_tok, arg_tok, 1, head))
-			add_arg_to_input_list(on_tok, arg_tok);
+		populate_template_value(template, in_tok);
+		if (check_if_different(template, in_tok, trailers, 1))
+			apply_template_to_trailers(template, on_tok);
 		else
-			free_arg_item(arg_tok);
+			free_template(template);
 		break;
 	case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
-		apply_item_command(in_tok, arg_tok);
-		if (check_if_different(on_tok, arg_tok, 0, head))
-			add_arg_to_input_list(on_tok, arg_tok);
+		populate_template_value(template, in_tok);
+		if (check_if_different(template, on_tok, trailers, 0))
+			apply_template_to_trailers(template, on_tok);
 		else
-			free_arg_item(arg_tok);
+			free_template(template);
 		break;
 	default:
 		BUG("trailer.c: unhandled value %d",
-		    arg_tok->conf.if_exists);
+		    template->conf.if_exists);
 	}
 }
 
-static void apply_arg_if_missing(struct list_head *head,
-				 struct arg_item *arg_tok)
+static void maybe_add_if_missing(struct trailer_template *template,
+				 struct list_head *trailers)
 {
 	enum trailer_where where;
 	struct trailer_item *to_add;
 
-	switch (arg_tok->conf.if_missing) {
+	switch (template->conf.if_missing) {
 	case MISSING_DO_NOTHING:
-		free_arg_item(arg_tok);
+		free_template(template);
 		break;
 	case MISSING_ADD:
-		where = arg_tok->conf.where;
-		apply_item_command(NULL, arg_tok);
-		to_add = trailer_from_arg(arg_tok);
+		where = template->conf.where;
+		populate_template_value(template, NULL);
+		to_add = trailer_from(template);
 		if (after_or_end(where))
-			list_add_tail(&to_add->list, head);
+			list_add_tail(&to_add->list, trailers);
 		else
-			list_add(&to_add->list, head);
+			list_add(&to_add->list, trailers);
 		break;
 	default:
 		BUG("trailer.c: unhandled value %d",
-		    arg_tok->conf.if_missing);
+		    template->conf.if_missing);
 	}
 }
 
-static int find_same_and_apply_arg(struct list_head *head,
-				   struct arg_item *arg_tok)
+static int find_same_and_apply_arg(struct trailer_template *template,
+				   struct list_head *trailers)
 {
 	struct list_head *pos;
 	struct trailer_item *in_tok;
 	struct trailer_item *on_tok;
 
-	enum trailer_where where = arg_tok->conf.where;
+	enum trailer_where where = template->conf.where;
 	int middle = (where == WHERE_AFTER) || (where == WHERE_BEFORE);
 	int backwards = after_or_end(where);
 	struct trailer_item *start_tok;
 
-	if (list_empty(head))
+	if (list_empty(trailers))
 		return 0;
 
-	start_tok = list_entry(backwards ? head->prev : head->next,
+	start_tok = list_entry(backwards ? trailers->prev : trailers->next,
 			       struct trailer_item,
 			       list);
 
-	list_for_each_dir(pos, head, backwards) {
+	list_for_each_dir(pos, trailers, backwards) {
 		in_tok = list_entry(pos, struct trailer_item, list);
-		if (!same_token(in_tok, arg_tok))
+		if (!same_token(in_tok, template))
 			continue;
 		on_tok = middle ? in_tok : start_tok;
-		apply_arg_if_exists(in_tok, arg_tok, on_tok, head);
+		maybe_add_if_exists(template, in_tok, on_tok, trailers);
 		return 1;
 	}
 	return 0;
 }
 
-void process_trailers_lists(struct list_head *head,
-			    struct list_head *arg_head)
+void apply_trailer_templates(struct list_head *templates,
+			     struct list_head *trailers)
 {
 	struct list_head *pos, *p;
-	struct arg_item *arg_tok;
+	struct trailer_template *template;
 
-	list_for_each_safe(pos, p, arg_head) {
+	list_for_each_safe(pos, p, templates) {
 		int applied = 0;
-		arg_tok = list_entry(pos, struct arg_item, list);
+		template = list_entry(pos, struct trailer_template, list);
 
 		list_del(pos);
 
-		applied = find_same_and_apply_arg(head, arg_tok);
+		applied = find_same_and_apply_arg(template, trailers);
 
 		if (!applied)
-			apply_arg_if_missing(head, arg_tok);
+			maybe_add_if_missing(template, trailers);
 	}
 }
 
@@ -458,26 +468,26 @@ void duplicate_trailer_conf(struct trailer_conf *dst,
 	dst->cmd = xstrdup_or_null(src->cmd);
 }
 
-static struct arg_item *get_conf_item(const char *name)
+static struct trailer_template *get_or_add_template_by(const char *name)
 {
 	struct list_head *pos;
-	struct arg_item *item;
+	struct trailer_template *template;
 
-	/* Look up item with same name */
-	list_for_each(pos, &conf_head) {
-		item = list_entry(pos, struct arg_item, list);
-		if (!strcasecmp(item->conf.name, name))
-			return item;
+	/* Look up template with same name. */
+	list_for_each(pos, &templates_from_conf) {
+		template = list_entry(pos, struct trailer_template, list);
+		if (!strcasecmp(template->conf.name, name))
+			return template;
 	}
 
-	/* Item does not already exists, create it */
-	CALLOC_ARRAY(item, 1);
-	duplicate_trailer_conf(&item->conf, &default_trailer_conf);
-	item->conf.name = xstrdup(name);
+	/* Template does not already exist; create it. */
+	CALLOC_ARRAY(template, 1);
+	duplicate_trailer_conf(&template->conf, &default_trailer_conf);
+	template->conf.name = xstrdup(name);
 
-	list_add_tail(&item->list, &conf_head);
+	list_add_tail(&template->list, &templates_from_conf);
 
-	return item;
+	return template;
 }
 
 enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_CMD,
@@ -535,7 +545,7 @@ static int git_trailer_config(const char *conf_key, const char *value,
 			      void *cb UNUSED)
 {
 	const char *trailer_item, *variable_name;
-	struct arg_item *item;
+	struct trailer_template *template;
 	struct trailer_conf *conf;
 	char *name = NULL;
 	enum trailer_info_type type;
@@ -560,8 +570,8 @@ static int git_trailer_config(const char *conf_key, const char *value,
 	if (!name)
 		return 0;
 
-	item = get_conf_item(name);
-	conf = &item->conf;
+	template = get_or_add_template_by(name);
+	conf = &template->conf;
 	free(name);
 
 	switch (type) {
@@ -618,20 +628,22 @@ void trailer_config_init(void)
 	configured = 1;
 }
 
-static const char *token_from_item(struct arg_item *item, char *tok)
+static const char *token_from_template(struct trailer_template *template, char *tok)
 {
-	if (item->conf.key)
-		return item->conf.key;
+	if (template->conf.key)
+		return template->conf.key;
 	if (tok)
 		return tok;
-	return item->conf.name;
+	return template->conf.name;
 }
 
-static int token_matches_item(const char *tok, struct arg_item *item, size_t tok_len)
+static int token_matches_template(const char *tok,
+				  struct trailer_template *template,
+				  size_t tok_len)
 {
-	if (!strncasecmp(tok, item->conf.name, tok_len))
+	if (!strncasecmp(tok, template->conf.name, tok_len))
 		return 1;
-	return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
+	return template->conf.key ? !strncasecmp(tok, template->conf.key, tok_len) : 0;
 }
 
 /*
@@ -677,7 +689,7 @@ void parse_trailer(const char *line, ssize_t separator_pos,
 		   struct strbuf *tok, struct strbuf *val,
 		   const struct trailer_conf **conf)
 {
-	struct arg_item *item;
+	struct trailer_template *template;
 	size_t tok_len;
 	struct list_head *pos;
 
@@ -695,13 +707,13 @@ void parse_trailer(const char *line, ssize_t separator_pos,
 	tok_len = token_len_without_separator(tok->buf, tok->len);
 	if (conf)
 		*conf = &default_trailer_conf;
-	list_for_each(pos, &conf_head) {
-		item = list_entry(pos, struct arg_item, list);
-		if (token_matches_item(tok->buf, item, tok_len)) {
+	list_for_each(pos, &templates_from_conf) {
+		template = list_entry(pos, struct trailer_template, list);
+		if (token_matches_template(tok->buf, template, tok_len)) {
 			char *tok_buf = strbuf_detach(tok, NULL);
 			if (conf)
-				*conf = &item->conf;
-			strbuf_addstr(tok, token_from_item(item, tok_buf));
+				*conf = &template->conf;
+			strbuf_addstr(tok, token_from_template(template, tok_buf));
 			free(tok_buf);
 			break;
 		}
@@ -718,29 +730,41 @@ static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
 	return new_item;
 }
 
-void trailer_add_arg_item(char *tok, char *val, const struct trailer_conf *conf,
-			  struct list_head *arg_head)
+void add_trailer_template(char *tok, char *val, const struct trailer_conf *conf,
+			  struct list_head *templates)
 {
-	struct arg_item *new_item = xcalloc(1, sizeof(*new_item));
-	new_item->token = tok;
-	new_item->value = val;
-	duplicate_trailer_conf(&new_item->conf, conf);
-	list_add_tail(&new_item->list, arg_head);
+	struct trailer_template *template = xcalloc(1, sizeof(*template));
+	template->token = tok;
+	template->value = val;
+	duplicate_trailer_conf(&template->conf, conf);
+	list_add_tail(&template->list, templates);
 }
 
-void parse_trailers_from_config(struct list_head *config_head)
+void parse_trailer_templates_from_config(struct list_head *config_head)
 {
-	struct arg_item *item;
+	struct trailer_template *template;
 	struct list_head *pos;
 
-	/* Add an arg item for each configured trailer with a command */
-	list_for_each(pos, &conf_head) {
-		item = list_entry(pos, struct arg_item, list);
-		if (item->conf.command)
-			trailer_add_arg_item(xstrdup(token_from_item(item, NULL)),
-					     xstrdup(""),
-					     &item->conf,
-					     config_head);
+	/*
+	 * Get configured templates with a ".command" option.
+	 *
+	 * NEEDSWORK: If the interpret-trailers builtin sees a
+	 * "trailer.foo.command = ..." setting, then the "foo" trailer will
+	 * always be inserted, even if "--trailer foo" is not provided.
+	 * Considering how ".command" is deprecated, it is a bit strange to see
+	 * it getting special treatment like this over ".cmd". Instead, we
+	 * should add a new option that explicitly lets the user decide if the
+	 * configured trailer should always be added automatically, or if it
+	 * should only be added if "--trailer foo" is provided (default).
+	 * Then we can collect configured trailers that have either ".command"
+	 * or ".cmd" below, instead of just ".command".
+	 */
+	list_for_each(pos, &templates_from_conf) {
+		template = list_entry(pos, struct trailer_template, list);
+		if (template->conf.command)
+			add_trailer_template(xstrdup(token_from_template(template,
+									 NULL)),
+					     xstrdup(""), &template->conf, config_head);
 	}
 }
 
@@ -852,7 +876,7 @@ static size_t find_trailer_block_start(const char *buf, size_t len)
 	 * Get the start of the trailers by looking starting from the end for a
 	 * blank line before a set of non-blank lines that (i) are all
 	 * trailers, or (ii) contains at least one Git-generated trailer and
-	 * consists of at least 25% trailers.
+	 * consists of at least 25% configured trailers.
 	 */
 	for (l = last_line(buf, len);
 	     l >= end_of_title;
@@ -896,10 +920,16 @@ static size_t find_trailer_block_start(const char *buf, size_t len)
 			possible_continuation_lines = 0;
 			if (recognized_prefix)
 				continue;
-			list_for_each(pos, &conf_head) {
-				struct arg_item *item;
-				item = list_entry(pos, struct arg_item, list);
-				if (token_matches_item(bol, item,
+			/*
+			 * The templates here are not used for actually
+			 * adding trailers anywhere, but instead to help us
+			 * identify trailer lines by comparing their keys with
+			 * those found in configured templates.
+			 */
+			list_for_each(pos, &templates_from_conf) {
+				struct trailer_template *template;
+				template = list_entry(pos, struct trailer_template, list);
+				if (token_matches_template(bol, template,
 						       separator_pos)) {
 					recognized_prefix = 1;
 					break;
@@ -1127,13 +1157,13 @@ void free_trailers(struct list_head *trailers)
 	}
 }
 
-void free_new_trailers(struct list_head *trailers)
+void free_trailer_templates(struct list_head *trailer_templates)
 {
 	struct list_head *pos, *p;
 
-	list_for_each_safe(pos, p, trailers) {
+	list_for_each_safe(pos, p, trailer_templates) {
 		list_del(pos);
-		free_arg_item(list_entry(pos, struct arg_item, list));
+		free_template(list_entry(pos, struct trailer_template, list));
 	}
 }
 
diff --git a/trailer.h b/trailer.h
index 5d4bacd9931..79aa123478d 100644
--- a/trailer.h
+++ b/trailer.h
@@ -43,8 +43,8 @@ void duplicate_trailer_conf(struct trailer_conf *dst,
 
 const char *trailer_default_separators(void);
 
-void trailer_add_arg_item(char *tok, char *val, const struct trailer_conf *conf,
-			  struct list_head *arg_head);
+void add_trailer_template(char *tok, char *val, const struct trailer_conf *conf,
+			  struct list_head *templates);
 
 struct process_trailer_options {
 	int in_place;
@@ -63,10 +63,9 @@ struct process_trailer_options {
 
 #define PROCESS_TRAILER_OPTIONS_INIT {0}
 
-void parse_trailers_from_config(struct list_head *config_head);
+void parse_trailer_templates_from_config(struct list_head *config_head);
 
-void process_trailers_lists(struct list_head *head,
-			    struct list_head *arg_head);
+void apply_trailer_templates(struct list_head *templates, struct list_head *trailers_head);
 
 ssize_t find_separator(const char *line, const char *separators);
 
@@ -89,7 +88,7 @@ void format_trailers(const struct process_trailer_options *opts,
 		     struct list_head *trailers,
 		     struct strbuf *out);
 void free_trailers(struct list_head *);
-void free_new_trailers(struct list_head *);
+void free_trailer_templates(struct list_head *);
 
 /*
  * Convenience function to format the trailers from the commit msg "msg" into
-- 
gitgitgadget
Previous: Linus ArverNext: Linus Arver via GitGitGadget
Message 93 of 202 in “Enrich Trailer API”
  1. 00/10 Enrich Trailer APILinus Arver via GitGitGadget, Jan 10, 2024
  2. 01/10 trailer: move process_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Jan 10, 2024
  3. Junio C HamanoJan 18, 2024
  4. Linus ArverJan 19, 2024
  5. 02/10 trailer: include "trailer" term in API functionsLinus Arver via GitGitGadget, Jan 10, 2024
  6. Junio C HamanoJan 18, 2024
  7. Linus ArverJan 19, 2024
  8. Junio C HamanoJan 19, 2024
  9. 03/10 trailer: unify trailer formatting machineryLinus Arver via GitGitGadget, Jan 10, 2024
  10. Junio C HamanoJan 18, 2024
  11. Linus ArverJan 19, 2024
  12. 04/10 trailer: delete obsolete formatting functionsLinus Arver via GitGitGadget, Jan 10, 2024
  13. Junio C HamanoJan 19, 2024
  14. 05/10 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Jan 10, 2024
  15. Junio C HamanoJan 19, 2024
  16. Linus ArverJan 20, 2024
  17. Linus ArverJan 22, 2024
  18. 06/10 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Jan 10, 2024
  19. Junio C HamanoJan 19, 2024
  20. Josh SteadmonJan 25, 2024
  21. Junio C HamanoJan 25, 2024
  22. 07/10 trailer: spread usage of "trailer_block" languageLinus Arver via GitGitGadget, Jan 10, 2024
  23. Junio C HamanoJan 19, 2024
  24. Linus ArverJan 20, 2024
  25. 08/10 trailer: prepare to move parse_trailers_from_command_line_args() to builtinLinus Arver via GitGitGadget, Jan 10, 2024
  26. Josh SteadmonJan 25, 2024
  27. Linus ArverJan 26, 2024
  28. 09/10 trailer: move arg handling to interpret-trailers.cLinus Arver via GitGitGadget, Jan 10, 2024
  29. Junio C HamanoJan 19, 2024
  30. Linus ArverJan 20, 2024
  31. 10/10 trailer: delete obsolete argument handling code from APILinus Arver via GitGitGadget, Jan 10, 2024
  32. Junio C HamanoJan 10, 2024
  33. Linus ArverJan 13, 2024
  34. Linus ArverJan 14, 2024
  35. Josh SteadmonJan 25, 2024
  36. 00/10 Enrich Trailer APILinus Arver via GitGitGadget, Jan 26, 2024
  37. 01/10 trailer: prepare to expose functions as part of APILinus Arver via GitGitGadget, Jan 26, 2024
  38. Josh SteadmonJan 30, 2024
  39. Linus ArverJan 30, 2024
  40. 02/10 trailer: move interpret_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Jan 26, 2024
  41. 03/10 trailer: unify trailer formatting machineryLinus Arver via GitGitGadget, Jan 26, 2024
  42. Josh SteadmonJan 30, 2024
  43. Linus ArverJan 30, 2024
  44. 04/10 trailer: delete obsolete formatting functionsLinus Arver via GitGitGadget, Jan 26, 2024
  45. 05/10 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Jan 26, 2024
  46. 06/10 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Jan 26, 2024
  47. 07/10 trailer: spread usage of "trailer_block" languageLinus Arver via GitGitGadget, Jan 26, 2024
  48. 08/10 trailer: prepare to move parse_trailers_from_command_line_args() to builtinLinus Arver via GitGitGadget, Jan 26, 2024
  49. 09/10 trailer: move arg handling to interpret-trailers.cLinus Arver via GitGitGadget, Jan 26, 2024
  50. Linus ArverJan 28, 2024
  51. Linus ArverJan 28, 2024
  52. 10/10 trailer: delete obsolete argument handling code from APILinus Arver via GitGitGadget, Jan 26, 2024
  53. 00/10 Enrich Trailer APILinus Arver via GitGitGadget, Jan 31, 2024
  54. 01/10 trailer: prepare to expose functions as part of APILinus Arver via GitGitGadget, Jan 31, 2024
  55. 02/10 trailer: move interpret_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Jan 31, 2024
  56. Junio C HamanoJan 31, 2024
  57. Linus ArverJan 31, 2024
  58. 03/10 trailer: unify trailer formatting machineryLinus Arver via GitGitGadget, Jan 31, 2024
  59. Josh SteadmonJan 31, 2024
  60. Linus ArverJan 31, 2024
  61. Junio C HamanoFeb 1, 2024
  62. Linus ArverFeb 1, 2024
  63. Junio C HamanoJan 31, 2024
  64. Junio C HamanoJan 31, 2024
  65. Linus ArverFeb 1, 2024
  66. Junio C HamanoFeb 1, 2024
  67. Junio C HamanoFeb 1, 2024
  68. Linus ArverFeb 1, 2024
  69. Junio C HamanoFeb 1, 2024
  70. Linus ArverFeb 2, 2024
  71. Junio C HamanoFeb 2, 2024
  72. Linus ArverJan 31, 2024
  73. 04/10 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Jan 31, 2024
  74. Junio C HamanoFeb 1, 2024
  75. Linus ArverFeb 1, 2024
  76. Linus ArverFeb 3, 2024
  77. 05/10 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Jan 31, 2024
  78. Junio C HamanoFeb 1, 2024
  79. Linus ArverFeb 3, 2024
  80. Junio C HamanoFeb 3, 2024
  81. 06/10 trailer: spread usage of "trailer_block" languageLinus Arver via GitGitGadget, Jan 31, 2024
  82. Junio C HamanoFeb 1, 2024
  83. Linus ArverFeb 3, 2024
  84. 07/10 trailer: prepare to move parse_trailers_from_command_line_args() to builtinLinus Arver via GitGitGadget, Jan 31, 2024
  85. Junio C HamanoFeb 1, 2024
  86. 08/10 trailer: move arg handling to interpret-trailers.cLinus Arver via GitGitGadget, Jan 31, 2024
  87. Junio C HamanoFeb 1, 2024
  88. Linus ArverFeb 3, 2024
  89. Linus ArverFeb 6, 2024
  90. 09/10 trailer: delete obsolete argument handling code from APILinus Arver via GitGitGadget, Jan 31, 2024
  91. Junio C HamanoFeb 1, 2024
  92. Linus ArverFeb 3, 2024
  93. 10/10 trailer: introduce "template" term for readabilityLinus Arver via GitGitGadget, Jan 31, 2024
  94. 00/28 Enrich Trailer APILinus Arver via GitGitGadget, Feb 6, 2024
  95. 01/28 trailer: free trailer_info _after_ all related usageLinus Arver via GitGitGadget, Feb 6, 2024
  96. 02/28 shortlog: add test for de-duplicating folded trailersLinus Arver via GitGitGadget, Feb 6, 2024
  97. 03/28 trailer: prepare to expose functions as part of APILinus Arver via GitGitGadget, Feb 6, 2024
  98. 04/28 trailer: move interpret_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Feb 6, 2024
  99. 05/28 trailer: start preparing for formatting unificationLinus Arver via GitGitGadget, Feb 6, 2024
  100. 06/28 trailer_info_get(): reorder parametersLinus Arver via GitGitGadget, Feb 6, 2024
  101. 07/28 format_trailers(): use strbuf instead of FILELinus Arver via GitGitGadget, Feb 6, 2024
  102. 08/28 format_trailer_info(): move "fast path" to callerLinus Arver via GitGitGadget, Feb 6, 2024
  103. 09/28 format_trailers_from_commit(): indirectly call trailer_info_get()Linus Arver via GitGitGadget, Feb 6, 2024
  104. 10/28 format_trailer_info(): use trailer_item objectsLinus Arver via GitGitGadget, Feb 6, 2024
  105. Junio C HamanoFeb 9, 2024
  106. Linus ArverFeb 13, 2024
  107. 11/28 format_trailer_info(): drop redundant unfold_value()Linus Arver via GitGitGadget, Feb 6, 2024
  108. Junio C HamanoFeb 9, 2024
  109. 12/28 format_trailer_info(): append newline for non-trailer linesLinus Arver via GitGitGadget, Feb 6, 2024
  110. Junio C HamanoFeb 9, 2024
  111. Christian CouderFeb 12, 2024
  112. Linus ArverFeb 13, 2024
  113. 13/28 trailer: begin formatting unificationLinus Arver via GitGitGadget, Feb 6, 2024
  114. 14/28 format_trailer_info(): teach it about opts->trim_emptyLinus Arver via GitGitGadget, Feb 6, 2024
  115. Christian CouderFeb 12, 2024
  116. Linus ArverFeb 13, 2024
  117. Christian CouderFeb 13, 2024
  118. 15/28 format_trailer_info(): avoid double-printing the separatorLinus Arver via GitGitGadget, Feb 6, 2024
  119. Christian CouderFeb 12, 2024
  120. Linus ArverFeb 13, 2024
  121. Christian CouderFeb 13, 2024
  122. Linus ArverFeb 13, 2024
  123. Linus ArverMar 15, 2024
  124. Kristoffer HaugsbakkFeb 13, 2024
  125. 16/28 trailer: finish formatting unificationLinus Arver via GitGitGadget, Feb 6, 2024
  126. Junio C HamanoFeb 9, 2024
  127. Christian CouderFeb 12, 2024
  128. Linus ArverFeb 13, 2024
  129. 17/28 trailer: teach iterator about non-trailer linesLinus Arver via GitGitGadget, Feb 6, 2024
  130. 18/28 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Feb 6, 2024
  131. 19/28 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Feb 6, 2024
  132. Junio C HamanoFeb 9, 2024
  133. Linus ArverFeb 13, 2024
  134. Christian CouderFeb 12, 2024
  135. Linus ArverFeb 13, 2024
  136. 20/28 trailer: retire trailer_info_get() from APILinus Arver via GitGitGadget, Feb 6, 2024
  137. 21/28 trailer: spread usage of "trailer_block" languageLinus Arver via GitGitGadget, Feb 6, 2024
  138. Christian CouderFeb 12, 2024
  139. Linus ArverFeb 13, 2024
  140. 22/28 trailer: prepare to delete "parse_trailers_from_command_line_args()"Linus Arver via GitGitGadget, Feb 6, 2024
  141. Christian CouderFeb 12, 2024
  142. Linus ArverFeb 13, 2024
  143. 23/28 trailer: add new helper functions to APILinus Arver via GitGitGadget, Feb 6, 2024
  144. Christian CouderFeb 12, 2024
  145. Linus ArverFeb 13, 2024
  146. 24/28 trailer_add_arg_item(): drop new_trailer_item usageLinus Arver via GitGitGadget, Feb 6, 2024
  147. 25/28 trailer: deprecate "new_trailer_item" struct from APILinus Arver via GitGitGadget, Feb 6, 2024
  148. 26/28 trailer: unify "--trailer ..." arg handlingLinus Arver via GitGitGadget, Feb 6, 2024
  149. Christian CouderFeb 12, 2024
  150. Linus ArverFeb 13, 2024
  151. 27/28 trailer_set_*(): put out parameter at the endLinus Arver via GitGitGadget, Feb 6, 2024
  152. Christian CouderFeb 12, 2024
  153. Linus ArverFeb 13, 2024
  154. 28/28 trailer: introduce "template" term for readabilityLinus Arver via GitGitGadget, Feb 6, 2024
  155. Christian CouderFeb 12, 2024
  156. Linus ArverFeb 13, 2024
  157. Christian CouderFeb 12, 2024
  158. Junio C HamanoFeb 13, 2024
  159. Christian CouderFeb 13, 2024
  160. Junio C HamanoFeb 13, 2024
  161. Christian CouderFeb 13, 2024
  162. Linus ArverFeb 16, 2024
  163. Linus ArverFeb 13, 2024
  164. Junio C HamanoFeb 13, 2024
  165. Kristoffer HaugsbakkFeb 13, 2024
  166. Christian CouderFeb 13, 2024
  167. Linus ArverFeb 16, 2024
  168. 0/9 Enrich Trailer APILinus Arver via GitGitGadget, Feb 16, 2024
  169. 1/9 trailer: free trailer_info _after_ all related usageLinus Arver via GitGitGadget, Feb 16, 2024
  170. 2/9 shortlog: add test for de-duplicating folded trailersLinus Arver via GitGitGadget, Feb 16, 2024
  171. 3/9 trailer: prepare to expose functions as part of APILinus Arver via GitGitGadget, Feb 16, 2024
  172. Christian CouderFeb 19, 2024
  173. Linus ArverFeb 29, 2024
  174. Junio C HamanoFeb 29, 2024
  175. Linus ArverFeb 29, 2024
  176. 4/9 trailer: move interpret_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Feb 16, 2024
  177. 5/9 trailer: start preparing for formatting unificationLinus Arver via GitGitGadget, Feb 16, 2024
  178. Christian CouderFeb 19, 2024
  179. Linus ArverFeb 29, 2024
  180. 6/9 trailer_info_get(): reorder parametersLinus Arver via GitGitGadget, Feb 16, 2024
  181. 7/9 format_trailers(): use strbuf instead of FILELinus Arver via GitGitGadget, Feb 16, 2024
  182. 8/9 format_trailer_info(): move "fast path" to callerLinus Arver via GitGitGadget, Feb 16, 2024
  183. 9/9 format_trailers_from_commit(): indirectly call trailer_info_get()Linus Arver via GitGitGadget, Feb 16, 2024
  184. Christian CouderFeb 19, 2024
  185. Linus ArverFeb 29, 2024
  186. Christian CouderFeb 19, 2024
  187. 0/9 Enrich Trailer APILinus Arver via GitGitGadget, Mar 1, 2024
  188. 1/9 trailer: free trailer_info _after_ all related usageLinus Arver via GitGitGadget, Mar 1, 2024
  189. 2/9 shortlog: add test for de-duplicating folded trailersLinus Arver via GitGitGadget, Mar 1, 2024
  190. 3/9 trailer: rename functions to use 'trailer'Linus Arver via GitGitGadget, Mar 1, 2024
  191. 4/9 trailer: move interpret_trailers() to interpret-trailers.cLinus Arver via GitGitGadget, Mar 1, 2024
  192. 5/9 trailer: reorder format_trailers_from_commit() parametersLinus Arver via GitGitGadget, Mar 1, 2024
  193. 6/9 trailer_info_get(): reorder parametersLinus Arver via GitGitGadget, Mar 1, 2024
  194. 7/9 format_trailers(): use strbuf instead of FILELinus Arver via GitGitGadget, Mar 1, 2024
  195. 8/9 format_trailer_info(): move "fast path" to callerLinus Arver via GitGitGadget, Mar 1, 2024
  196. 9/9 format_trailers_from_commit(): indirectly call trailer_info_get()Linus Arver via GitGitGadget, Mar 1, 2024
  197. Junio C HamanoMar 5, 2024
  198. Josh SteadmonMar 5, 2024
  199. Junio C HamanoMar 5, 2024
  200. Christian CouderMar 6, 2024
  201. Junio C HamanoMar 6, 2024
  202. Junio C HamanoMar 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.