Re: [PATCH 3/5] format_trailer_info(): append newline for non-trailer lines

[Junio C Hamano <gitster@pobox.com>]

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Linus Arver <linusa@google.com>
>
> This wraps up the preparatory refactors to unify the trailer formatters.
> ...
> The test suite can pass again without the need to hide failures
> with *_failure, so flip the affected test cases back to *_success. Now,
> format_trailer_info() is in better shape to supersede format_trailers(),
> which we'll do in the next commit.

Nicely done.  Queued.

>
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
>  t/t4205-log-pretty-formats.sh | 12 ++++++------
>  t/t6300-for-each-ref.sh       | 16 ++--------------
>  trailer.c                     |  5 +++--
>  3 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index 339e0c892ef..e3d655e6b8b 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -675,7 +675,7 @@ test_expect_success '%(trailers:only=no,only=true) shows only "key: value" trail
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure '%(trailers:unfold) unfolds trailers' '
> +test_expect_success '%(trailers:unfold) unfolds trailers' '
>  	git log --no-walk --pretty="%(trailers:unfold)" >actual &&
>  	{
>  		unfold <trailers &&
> @@ -737,7 +737,7 @@ test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
> +test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
>  	git log --no-walk --pretty="format:%(trailers:key=Acked-by,only=no)" >actual &&
>  	{
>  		echo "Acked-by: A U Thor <author@example.com>" &&
> @@ -752,7 +752,7 @@ test_expect_success '%(trailers:key) without value is error' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure '%(trailers:keyonly) shows only keys' '
> +test_expect_success '%(trailers:keyonly) shows only keys' '
>  	git log --no-walk --pretty="format:%(trailers:keyonly)" >actual &&
>  	test_write_lines \
>  		"Signed-off-by" \
> @@ -774,7 +774,7 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure '%(trailers:valueonly) shows only values' '
> +test_expect_success '%(trailers:valueonly) shows only values' '
>  	git log --no-walk --pretty="format:%(trailers:valueonly)" >actual &&
>  	test_write_lines \
>  		"A U Thor <author@example.com>" \
> @@ -813,7 +813,7 @@ test_expect_success 'pretty format %(trailers:separator=X,unfold) changes separa
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-value separator' '
> +test_expect_success 'pretty format %(trailers:key_value_separator) changes key-value separator' '
>  	git log --no-walk --pretty=format:"X%(trailers:key_value_separator=%x00)X" >actual &&
>  	(
>  		printf "XSigned-off-by\0A U Thor <author@example.com>\n" &&
> @@ -824,7 +824,7 @@ test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-v
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '
> +test_expect_success 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '
>  	git log --no-walk --pretty=format:"X%(trailers:key_value_separator=%x00,unfold)X" >actual &&
>  	(
>  		printf "XSigned-off-by\0A U Thor <author@example.com>\n" &&
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 2688dcc7b9e..eb6c8204e8b 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -1446,19 +1446,7 @@ test_trailer_option () {
>  	'
>  }
>  
> -# Just like test_trailer_option, but expect failure instead of success.
> -test_trailer_option_expect_failure () {
> -	title=$1 option=$2
> -	cat >expect
> -	test_expect_failure "$title" '
> -		git for-each-ref --format="%($option)" refs/heads/main >actual &&
> -		test_cmp expect actual &&
> -		git for-each-ref --format="%(contents:$option)" refs/heads/main >actual &&
> -		test_cmp expect actual
> -	'
> -}
> -
> -test_trailer_option_expect_failure '%(trailers:unfold) unfolds trailers' \
> +test_trailer_option '%(trailers:unfold) unfolds trailers' \
>  	'trailers:unfold' <<-EOF
>  	$(unfold <trailers)
>  
> @@ -1542,7 +1530,7 @@ test_trailer_option '%(trailers:key=foo,unfold) properly unfolds' \
>  
>  	EOF
>  
> -test_trailer_option_expect_failure '%(trailers:key=foo,only=no) also includes nontrailer lines' \
> +test_trailer_option '%(trailers:key=foo,only=no) also includes nontrailer lines' \
>  	'trailers:key=Signed-off-by,only=no' <<-EOF
>  	Signed-off-by: A U Thor <author@example.com>
>  	$(grep patch.description <trailers)
> diff --git a/trailer.c b/trailer.c
> index 2c0dd8ac829..fe8b0819d55 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1124,9 +1124,10 @@ static void format_trailer_info(const struct process_trailer_options *opts,
>  				strbuf_addbuf(out, opts->separator);
>  			}
>  			strbuf_addstr(out, item->value);
> -			if (opts->separator) {
> +			if (opts->separator)
>  				strbuf_rtrim(out);
> -			}
> +			else
> +				strbuf_addch(out, '\n');
>  		}
>  	}
>  }