Re: [PATCH v4 14/28] format_trailer_info(): teach it about opts->trim_empty
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Feb 12, 2024, 23:38 UTC
- Message-ID
- <CAP8UFD3wMD5SmfpP20jST4YndBdPX9U9qAbKh=4TnUzLkpULRA@mail.gmail.com>
- In-Reply-To
- <11f854399db2b0da5d82cad910c3b86ca9c2e0db.1707196348.git.gitgitgadget@gmail.com>
Show 21 quoted lines
> diff --git a/trailer.c b/trailer.c
> index f4defad3dae..c28b6c11cc5 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -162,20 +162,6 @@ static void print_tok_val(struct strbuf *out, const char *tok, const char *val)
> strbuf_addf(out, "%s%c %s\n", tok, separators[0], val);
> }
>
> -static 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);
> - }
> -}It seems to me that this function could and should have been removed in the previous patch. If there is a reason why it is better to do it in this patch, I think it should be explained more clearly in the commit message.
Show 16 quoted lines
> static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
> {
> struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
> @@ -1101,6 +1087,15 @@ 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;
> +Wasn't it possible to make this change in format_trailer_info() before using format_trailer_info() to replace format_trailers()?
> if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
> if (opts->separator && out->len != origlen)
> strbuf_addbuf(out, opts->separator);