trailer: begin formatting unification

Now that the preparatory refactors are over, we can replace the call to format_trailers() in interpret-trailers with format_trailer_info(). This unifies the trailer formatting machinery In order to avoid breakages in t7502 and t7513, we have to steal the features present in format_trailers(). Namely, we have to teach format_trailer_info() as follows: (1) make it aware of opts->trim_empty, and (2) make it avoid hardcoding ": " as the separator and space (which can result in double-printing these characters). For (2), make it only print the separator and space if we cannot find any recognized separator somewhere in the key (yes, keys may have a trailing separator in it --- we will eventually fix this design but not now). Do so by copying the code out of print_tok_val(), and deleting the same function. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Linus Arver committed Mar 15, 2024 at 06:55 UTC 676c1db76e310c400b602890ac6853fdf8fdfa98
3 files changed +19 -39
builtin/interpret-trailers.c
+1 -1
@@ -171,7 +171,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
171 }
172
173 /* Print trailer block. */
174 - format_trailers(opts, &head, &trailer_block);
174 + format_trailer_info(opts, &head, &trailer_block);
175 free_trailers(&head);
176 fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
177 strbuf_release(&trailer_block);
trailer.c
+17 -37
@@ -144,38 +144,6 @@ static char last_non_space_char(const char *s)
144 return '\0';
145 }
146
147 -static void print_tok_val(struct strbuf *out, const char *tok, const char *val)
148 -{
149 - char c;
150 -
151 - if (!tok) {
152 - strbuf_addf(out, "%s\n", val);
153 - return;
154 - }
155 -
156 - c = last_non_space_char(tok);
157 - if (!c)
158 - return;
159 - if (strchr(separators, c))
160 - strbuf_addf(out, "%s%s\n", tok, val);
161 - else
162 - strbuf_addf(out, "%s%c %s\n", tok, separators[0], val);
163 -}
164 -
165 -void format_trailers(const struct process_trailer_options *opts,
166 - struct list_head *trailers,
167 - struct strbuf *out)
168 -{
169 - struct list_head *pos;
170 - struct trailer_item *item;
171 - list_for_each(pos, trailers) {
172 - item = list_entry(pos, struct trailer_item, list);
173 - if ((!opts->trim_empty || strlen(item->value) > 0) &&
174 - (!opts->only_trailers || item->token))
175 - print_tok_val(out, item->token, item->value);
176 - }
177 -}
178 -
147 static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
148 {
149 struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
@@ -1084,9 +1052,9 @@ void trailer_info_release(struct trailer_info *info)
1052 free(info->trailers);
1053 }
1054
1087 -static void format_trailer_info(const struct process_trailer_options *opts,
1088 - struct list_head *trailers,
1089 - struct strbuf *out)
1055 +void format_trailer_info(const struct process_trailer_options *opts,
1056 + struct list_head *trailers,
1057 + struct strbuf *out)
1058 {
1059 size_t origlen = out->len;
1060 struct list_head *pos;
@@ -1100,6 +1068,15 @@ static void format_trailer_info(const struct process_trailer_options *opts,
1068 strbuf_addstr(&tok, item->token);
1069 strbuf_addstr(&val, item->value);
1070
1071 + /*
1072 + * Skip key/value pairs where the value was empty. This
1073 + * can happen from trailers specified without a
1074 + * separator, like `--trailer "Reviewed-by"` (no
1075 + * corresponding value).
1076 + */
1077 + if (opts->trim_empty && !strlen(item->value))
1078 + continue;
1079 +
1080 if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
1081 if (opts->separator && out->len != origlen)
1082 strbuf_addbuf(out, opts->separator);
@@ -1108,8 +1085,11 @@ static void format_trailer_info(const struct process_trailer_options *opts,
1085 if (!opts->key_only && !opts->value_only) {
1086 if (opts->key_value_separator)
1087 strbuf_addbuf(out, opts->key_value_separator);
1111 - else
1112 - strbuf_addstr(out, ": ");
1088 + else {
1089 + char c = last_non_space_char(tok.buf);
1090 + if (c && !strchr(separators, c))
1091 + strbuf_addf(out, "%c ", separators[0]);
1092 + }
1093 }
1094 if (!opts->key_only)
1095 strbuf_addbuf(out, &val);
trailer.h
+1 -1
@@ -101,7 +101,7 @@ void trailer_info_get(const struct process_trailer_options *,
101 void trailer_info_release(struct trailer_info *info);
102
103 void trailer_config_init(void);
104 -void format_trailers(const struct process_trailer_options *,
104 +void format_trailer_info(const struct process_trailer_options *,
105 struct list_head *trailers,
106 struct strbuf *out);
107 void free_trailers(struct list_head *);