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>

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 11f4ce9e4a274c666fe4127a904872653d35010d..f57af0db37b570d9d29c9107d6fcfbf71d78d4c7 100644 (file)
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -171,7 +171,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
        }
 
        /* Print trailer block. */
-       format_trailers(opts, &head, &trailer_block);
+       format_trailer_info(opts, &head, &trailer_block);
        free_trailers(&head);
        fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
        strbuf_release(&trailer_block);

diff --git a/trailer.c b/trailer.c
index fe8b0819d558cb2c411bd70b41ad3b2aeb9fde0d..43d5baef9ce8b5cb6c911634eea3380796d96ad0 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -144,38 +144,6 @@ static char last_non_space_char(const char *s)
        return '\0';
 }
 
-static void print_tok_val(struct strbuf *out, const char *tok, const char *val)
-{
-       char c;
-
-       if (!tok) {
-               strbuf_addf(out, "%s\n", val);
-               return;
-       }
-
-       c = last_non_space_char(tok);
-       if (!c)
-               return;
-       if (strchr(separators, c))
-               strbuf_addf(out, "%s%s\n", tok, val);
-       else
-               strbuf_addf(out, "%s%c %s\n", tok, separators[0], val);
-}
-
-void format_trailers(const struct process_trailer_options *opts,
-                    struct list_head *trailers,
-                    struct strbuf *out)
-{
-       struct list_head *pos;
-       struct trailer_item *item;
-       list_for_each(pos, trailers) {
-               item = list_entry(pos, struct trailer_item, list);
-               if ((!opts->trim_empty || strlen(item->value) > 0) &&
-                   (!opts->only_trailers || item->token))
-                       print_tok_val(out, item->token, item->value);
-       }
-}
-
 static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
 {
        struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
@@ -1084,9 +1052,9 @@ void trailer_info_release(struct trailer_info *info)
        free(info->trailers);
 }
 
-static void format_trailer_info(const struct process_trailer_options *opts,
-                               struct list_head *trailers,
-                               struct strbuf *out)
+void format_trailer_info(const struct process_trailer_options *opts,
+                        struct list_head *trailers,
+                        struct strbuf *out)
 {
        size_t origlen = out->len;
        struct list_head *pos;
@@ -1100,6 +1068,15 @@ static void format_trailer_info(const struct process_trailer_options *opts,
                        strbuf_addstr(&tok, item->token);
                        strbuf_addstr(&val, item->value);
 
+                       /*
+                        * Skip key/value pairs where the value was empty. This
+                        * can happen from trailers specified without a
+                        * separator, like `--trailer "Reviewed-by"` (no
+                        * corresponding value).
+                        */
+                       if (opts->trim_empty && !strlen(item->value))
+                               continue;
+
                        if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
                                if (opts->separator && out->len != origlen)
                                        strbuf_addbuf(out, opts->separator);
@@ -1108,8 +1085,11 @@ static void format_trailer_info(const struct process_trailer_options *opts,
                                if (!opts->key_only && !opts->value_only) {
                                        if (opts->key_value_separator)
                                                strbuf_addbuf(out, opts->key_value_separator);
-                                       else
-                                               strbuf_addstr(out, ": ");
+                                       else {
+                                               char c = last_non_space_char(tok.buf);
+                                               if (c && !strchr(separators, c))
+                                                       strbuf_addf(out, "%c ", separators[0]);
+                                       }
                                }
                                if (!opts->key_only)
                                        strbuf_addbuf(out, &val);

diff --git a/trailer.h b/trailer.h
index 1d106b6dd403b7755e4c81a1a40c1eea8eeae1d6..3c13006a4c193aa9833be0f028f89d61f20326e6 100644 (file)
--- a/trailer.h
+++ b/trailer.h
@@ -101,7 +101,7 @@ void trailer_info_get(const struct process_trailer_options *,
 void trailer_info_release(struct trailer_info *info);
 
 void trailer_config_init(void);
-void format_trailers(const struct process_trailer_options *,
+void format_trailer_info(const struct process_trailer_options *,
                     struct list_head *trailers,
                     struct strbuf *out);
 void free_trailers(struct list_head *);