format_trailer_info(): append newline for non-trailer lines

This wraps up the preparatory refactors to unify the trailer formatters. Two patches ago we made format_trailer_info() use trailer_item objects instead of the "trailers" string array. The strings in the array include trailing newlines, because the string array is split up with trailer_lines = strbuf_split_buf(str + trailer_block_start, end_of_log_message - trailer_block_start, '\n', 0); in trailer_info_get() and strbuf_split_buf() includes the terminator (in this case the newline character '\n') for each split-up substring. And before we made the transition to use trailer_item objects for it, format_trailer_info() called parse_trailer() (which trims newlines) for trailer lines but did _not_ call parse_trailer() for non-trailer lines. So for trailer lines it had to add back the trimmed newline like this if (!opts->separator) strbuf_addch(out, '\n'); But for non-trailer lines it didn't have to add back the newline because it could just reuse same string in the "trailers" string array (which again, already included the trailing newline). Now that format_trailer_info() uses trailer_item objects for all cases, it can't rely on "trailers" string array anymore. And so it must be taught to add a newline back when printing non-trailer lines, just like it already does for trailer lines. Do so now. 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. 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 9f0c9702de9c87aa30697d88c4dc9ad0610f4201
3 files changed +11 -22
t/t4205-log-pretty-formats.sh
+6 -6
@@ -675,7 +675,7 @@ test_expect_success '%(trailers:only=no,only=true) shows only "key: value" trail
675 test_cmp expect actual
676 '
677
678 -test_expect_failure '%(trailers:unfold) unfolds trailers' '
678 +test_expect_success '%(trailers:unfold) unfolds trailers' '
679 git log --no-walk --pretty="%(trailers:unfold)" >actual &&
680 {
681 unfold <trailers &&
@@ -737,7 +737,7 @@ test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '
737 test_cmp expect actual
738 '
739
740 -test_expect_failure 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
740 +test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
741 git log --no-walk --pretty="format:%(trailers:key=Acked-by,only=no)" >actual &&
742 {
743 echo "Acked-by: A U Thor <author@example.com>" &&
@@ -752,7 +752,7 @@ test_expect_success '%(trailers:key) without value is error' '
752 test_cmp expect actual
753 '
754
755 -test_expect_failure '%(trailers:keyonly) shows only keys' '
755 +test_expect_success '%(trailers:keyonly) shows only keys' '
756 git log --no-walk --pretty="format:%(trailers:keyonly)" >actual &&
757 test_write_lines \
758 "Signed-off-by" \
@@ -774,7 +774,7 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
774 test_cmp expect actual
775 '
776
777 -test_expect_failure '%(trailers:valueonly) shows only values' '
777 +test_expect_success '%(trailers:valueonly) shows only values' '
778 git log --no-walk --pretty="format:%(trailers:valueonly)" >actual &&
779 test_write_lines \
780 "A U Thor <author@example.com>" \
@@ -813,7 +813,7 @@ test_expect_success 'pretty format %(trailers:separator=X,unfold) changes separa
813 test_cmp expect actual
814 '
815
816 -test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-value separator' '
816 +test_expect_success 'pretty format %(trailers:key_value_separator) changes key-value separator' '
817 git log --no-walk --pretty=format:"X%(trailers:key_value_separator=%x00)X" >actual &&
818 (
819 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
824 test_cmp expect actual
825 '
826
827 -test_expect_failure 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '
827 +test_expect_success 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '
828 git log --no-walk --pretty=format:"X%(trailers:key_value_separator=%x00,unfold)X" >actual &&
829 (
830 printf "XSigned-off-by\0A U Thor <author@example.com>\n" &&
t/t6300-for-each-ref.sh
+2 -14
@@ -1446,19 +1446,7 @@ test_trailer_option () {
1446 '
1447 }
1448
1449 -# Just like test_trailer_option, but expect failure instead of success.
1450 -test_trailer_option_expect_failure () {
1451 - title=$1 option=$2
1452 - cat >expect
1453 - test_expect_failure "$title" '
1454 - git for-each-ref --format="%($option)" refs/heads/main >actual &&
1455 - test_cmp expect actual &&
1456 - git for-each-ref --format="%(contents:$option)" refs/heads/main >actual &&
1457 - test_cmp expect actual
1458 - '
1459 -}
1460 -
1461 -test_trailer_option_expect_failure '%(trailers:unfold) unfolds trailers' \
1449 +test_trailer_option '%(trailers:unfold) unfolds trailers' \
1450 'trailers:unfold' <<-EOF
1451 $(unfold <trailers)
1452
@@ -1542,7 +1530,7 @@ test_trailer_option '%(trailers:key=foo,unfold) properly unfolds' \
1530
1531 EOF
1532
1545 -test_trailer_option_expect_failure '%(trailers:key=foo,only=no) also includes nontrailer lines' \
1533 +test_trailer_option '%(trailers:key=foo,only=no) also includes nontrailer lines' \
1534 'trailers:key=Signed-off-by,only=no' <<-EOF
1535 Signed-off-by: A U Thor <author@example.com>
1536 $(grep patch.description <trailers)
trailer.c
+3 -2
@@ -1124,9 +1124,10 @@ static void format_trailer_info(const struct process_trailer_options *opts,
1124 strbuf_addbuf(out, opts->separator);
1125 }
1126 strbuf_addstr(out, item->value);
1127 - if (opts->separator) {
1127 + if (opts->separator)
1128 strbuf_rtrim(out);
1129 - }
1129 + else
1130 + strbuf_addch(out, '\n');
1131 }
1132 }
1133 }