Re: [PATCH v2 3/3] ref-filter: use pretty.c logic for trailers

> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 2962f85a502..ea1f8417176 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -260,11 +260,40 @@ contents:lines=N::
>  	The first `N` lines of the message.
>  
>  Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]
> -are obtained as `trailers` (or by using the historical alias
> -`contents:trailers`).  Non-trailer lines from the trailer block can be omitted
> -with `trailers:only`. Whitespace-continuations can be removed from trailers so
> -that each trailer appears on a line by itself with its full content with
> -`trailers:unfold`. Both can be used together as `trailers:unfold,only`.
> +are obtained as `trailers[:options]` (or by using the historical alias
> +`contents:trailers[:options]`). Valid [:option] are:
> +** 'key=<K>': only show trailers with specified key. Matching is done
> +   case-insensitively and trailing colon is optional. If option is
> +   given multiple times trailer lines matching any of the keys are
> +   shown. This option automatically enables the `only` option so that
> +   non-trailer lines in the trailer block are hidden. If that is not
> +   desired it can be disabled with `only=false`.  E.g.,
> +   `%(trailers:key=Reviewed-by)` shows trailer lines with key
> +   `Reviewed-by`.
> +** 'only[=val]': select whether non-trailer lines from the trailer
> +   block should be included. The `only` keyword may optionally be
> +   followed by an equal sign and one of `true`, `on`, `yes` to omit or
> +   `false`, `off`, `no` to show the non-trailer lines. If option is
> +   given without value it is enabled. If given multiple times the last
> +   value is used.
> +** 'separator=<SEP>': specify a separator inserted between trailer
> +   lines. When this option is not given each trailer line is
> +   terminated with a line feed character. The string SEP may contain
> +   the literal formatting codes. To use comma as separator one must use
> +   `%x2C` as it would otherwise be parsed as next option. If separator
> +   option is given multiple times only the last one is used.
> +   E.g., `%(trailers:key=Ticket,separator=%x2C)` shows all trailer lines
> +   whose key is "Ticket" separated by a comma.
> +** 'key_value_separator=<SEP>': specify a separator inserted between
> +   key and value. The string SEP may contain the literal formatting codes.
> +   E.g., `%(trailers:key=Ticket,key_value_separator=%x2C)` shows all trailer
> +   lines whose key is "Ticket" with key and value separated by a comma.
> +** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
> +   option was given. In same way as to for `only` it can be followed
> +   by an equal sign and explicit value. E.g.,
> +   `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
> +** 'valueonly[=val]': skip over the key part of the trailer line and only
> +   show the value part. Also this optionally allows explicit value.

> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index ca62e764b58..a8835b13915 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -825,14 +825,32 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success '%(trailers:only) shows only "key: value" trailers' '
> +test_show_key_value_trailers () {
> +	option="$1"
> +	test_expect_success "%($option) shows only 'key: value' trailers" '
> +		{
> +			grep -v patch.description <trailers &&
> +			echo
> +		} >expect &&
> +		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_show_key_value_trailers 'trailers:only'
> +test_show_key_value_trailers 'trailers:only=no,only=true'
> +test_show_key_value_trailers 'trailers:only=yes'
> +
> +test_expect_success '%(trailers:only=no) shows all trailers' '
>  	{
> -		grep -v patch.description <trailers &&
> +		cat trailers &&
>  		echo
>  	} >expect &&
> -	git for-each-ref --format="%(trailers:only)" refs/heads/main >actual &&
> +	git for-each-ref --format="%(trailers:only=no)" refs/heads/main >actual &&
>  	test_cmp expect actual &&
> -	git for-each-ref --format="%(contents:trailers:only)" refs/heads/main >actual &&
> +	git for-each-ref --format="%(contents:trailers:only=no)" refs/heads/main >actual &&
>  	test_cmp expect actual
>  '
>  
> @@ -851,17 +869,92 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '
>  	test_cmp actual actual
>  '
>  
> -test_expect_success '%(trailers) rejects unknown trailers arguments' '
> -	# error message cannot be checked under i18n
> -	cat >expect <<-EOF &&
> -	fatal: unknown %(trailers) argument: unsupported
> -	EOF
> -	test_must_fail git for-each-ref --format="%(trailers:unsupported)" 2>actual &&
> -	test_i18ncmp expect actual &&
> -	test_must_fail git for-each-ref --format="%(contents:trailers:unsupported)" 2>actual &&
> -	test_i18ncmp expect actual
> +test_trailer_option() {
> +	title="$1"
> +	option="$2"
> +	expect="$3"
> +	test_expect_success "$title" '
> +		echo $expect >expect &&
> +		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 '%(trailers:key=foo) shows that trailer' \
> +	'trailers:key=Signed-off-by' 'Signed-off-by: A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:key=foo) is case insensitive' \
> +	'trailers:key=SiGned-oFf-bY' 'Signed-off-by: A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:key=foo:) trailing colon also works' \
> +	'trailers:key=Signed-off-by:' 'Signed-off-by: A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:key=foo) multiple keys' \
> +	'trailers:key=Reviewed-by:,key=Signed-off-by' 'Reviewed-by: A U Thor <author@example.com>\nSigned-off-by: A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:key=nonexistent) becomes empty' \
> +	'trailers:key=Shined-off-by:' ''
> +
> +test_expect_success '%(trailers:key=foo) handles multiple lines even if folded' '
> +	{
> +		grep -v patch.description <trailers | grep -v Signed-off-by | grep -v Reviewed-by &&
> +		echo
> +	} >expect &&
> +	git for-each-ref --format="%(trailers:key=Acked-by)" refs/heads/main >actual &&
> +	test_cmp expect actual &&
> +	git for-each-ref --format="%(contents:trailers:key=Acked-by)" refs/heads/main >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '
> +	{
> +		unfold <trailers | grep Signed-off-by &&
> +		echo
> +	} >expect &&
> +	git for-each-ref --format="%(trailers:key=Signed-Off-by,unfold)" refs/heads/main >actual &&
> +	test_cmp expect actual &&
> +	git for-each-ref --format="%(contents:trailers:key=Signed-Off-by,unfold)" refs/heads/main >actual &&
> +	test_cmp expect actual
>  '
>  
> +test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
> +	{
> +		echo "Signed-off-by: A U Thor <author@example.com>" &&
> +		grep patch.description <trailers &&
> +		echo
> +	} >expect &&
> +	git for-each-ref --format="%(trailers:key=Signed-off-by,only=no)" refs/heads/main >actual &&
> +	test_cmp expect actual &&
> +	git for-each-ref --format="%(contents:trailers:key=Signed-off-by,only=no)" refs/heads/main >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_trailer_option '%(trailers:key=foo,valueonly) shows only value' \
> +	'trailers:key=Signed-off-by,valueonly' 'A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:separator) changes separator' \
> +	'trailers:separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by: A U Thor <author@example.com>,Signed-off-by: A U Thor <author@example.com>'7
> +test_trailer_option '%(trailers:key_value_separator) changes key-value separator' \
> +	'trailers:key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by,A U Thor <author@example.com>\nSigned-off-by,A U Thor <author@example.com>\n'
> +test_trailer_option '%(trailers:separator,key_value_separator) changes both separators' \
> +	'trailers:separator=%x2C,key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by,A U Thor <author@example.com>,Signed-off-by,A U Thor <author@example.com>'
> +
> +test_failing_trailer_option () {
> +	title="$1"
> +	option="$2"
> +	error="$3"
> +	test_expect_success "$title" '
> +		# error message cannot be checked under i18n
> +		echo $error >expect &&
> +		test_must_fail git for-each-ref --format="%($option)" refs/heads/main 2>actual &&
> +		test_i18ncmp expect actual &&
> +		test_must_fail git for-each-ref --format="%(contents:$option)" refs/heads/main 2>actual &&
> +		test_i18ncmp expect actual
> +	'
> +}
> +
> +test_failing_trailer_option '%(trailers:key) without value is error' \
> +	'trailers:key' 'fatal: expected %(trailers:key=<value>)'
> +test_failing_trailer_option '%(trailers) rejects unknown trailers arguments' \
> +	'trailers:unsupported' 'fatal: unknown %(trailers) argument: unsupported'
> +
>  test_expect_success 'if arguments, %(contents:trailers) shows error if colon is missing' '
>  	cat >expect <<-EOF &&
>  	fatal: unrecognized %(contents) argument: trailersonly