Subject: Re: [PATCH v4 10/28] format_trailer_info(): use trailer_item objects

From: Junio C Hamano <[email-masked]>
Date: Fri, 09 Feb 2024 13:53:57 -0800

"Linus Arver via GitGitGadget" <[email-masked]> writes:

> From: Linus Arver <[email-masked]>
>
> This is another preparatory refactor to unify the trailer formatters.
>
> Make format_trailer_info() operate on trailer_item objects, not the raw
> string array.
>
> This breaks t4205 and t6300. We will continue to make improvements until
> the test suite passes again, ultimately renaming format_trailer_info()
> to format_trailers(), at which point the unification of these formatters
> will be complete.
>
> Signed-off-by: Linus Arver <[email-masked]>
> ---
>  trailer.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)

I would have expected a (tentative) flip from test_expect_success to
test_expect_failure in the affected tests, to illustrate what behaviour
change this step introduces and why.

But the huge single step in the previous round broken out into
smaller steps like this round makes them much easier to follow.

Thanks.

> diff --git a/trailer.c b/trailer.c
> index e6665c99cc3..6333dfe1c11 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1086,21 +1086,21 @@ void trailer_info_release(struct trailer_info *info)
>  }
>  
>  static void format_trailer_info(const struct process_trailer_options *opts,
> -				const struct trailer_info *info,
> +				struct list_head *trailers,
>  				struct strbuf *out)
>  {
>  	size_t origlen = out->len;
> -	size_t i;
> -
> -	for (i = 0; i < info->trailer_nr; i++) {
> -		char *trailer = info->trailers[i];
> -		ssize_t separator_pos = find_separator(trailer, separators);
> +	struct list_head *pos;
> +	struct trailer_item *item;
>  
> -		if (separator_pos >= 1) {
> +	list_for_each(pos, trailers) {
> +		item = list_entry(pos, struct trailer_item, list);
> +		if (item->token) {
>  			struct strbuf tok = STRBUF_INIT;
>  			struct strbuf val = STRBUF_INIT;
> +			strbuf_addstr(&tok, item->token);
> +			strbuf_addstr(&val, item->value);
>  
> -			parse_trailer(&tok, &val, NULL, trailer, separator_pos);
>  			if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
>  				if (opts->unfold)
>  					unfold_value(&val);
> @@ -1127,13 +1127,12 @@ static void format_trailer_info(const struct process_trailer_options *opts,
>  			if (opts->separator && out->len != origlen) {
>  				strbuf_addbuf(out, opts->separator);
>  			}
> -			strbuf_addstr(out, trailer);
> +			strbuf_addstr(out, item->value);
>  			if (opts->separator) {
>  				strbuf_rtrim(out);
>  			}
>  		}
>  	}
> -
>  }
>  
>  void format_trailers_from_commit(const struct process_trailer_options *opts,
> @@ -1152,7 +1151,7 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,
>  		strbuf_add(out, msg + info.trailer_block_start,
>  			   info.trailer_block_end - info.trailer_block_start);
>  	} else
> -		format_trailer_info(opts, &info, out);
> +		format_trailer_info(opts, &trailers, out);
>  
>  	free_trailers(&trailers);
>  	trailer_info_release(&info);