From: Linus Arver <linusa@google.com>
Date: Sat, 16 Mar 2024 06:53:30 GMT
Subject: Re: [PATCH] docs: correct trailer `key_value_separator` description
Message-ID: <owly1q8a4qhh.fsf@fine.c.googlers.com>
In-Reply-To: <20240316035612.752910-1-brianmlyles@gmail.com>

Brian Lyles <brianmlyles@gmail.com> writes:

> The description for `key_value_separator` incorrectly states that this
> separator is inserted between trailer lines, which appears likely to
> have been incorrectly copied from `separator` when this option was
> added.
>
> Update the description to correctly indicate that it is a separator that
> appears between the key and the value of each trailer.
>
> Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
> ---
>  Documentation/pretty-formats.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index d38b4ab566..4839c2843c 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -329,9 +329,9 @@ multiple times, the last occurrence wins.
>     `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
>  ** 'keyonly[=<bool>]': only show the key part of the trailer.
>  ** 'valueonly[=<bool>]': only show the value part of the trailer.
> -** 'key_value_separator=<sep>': specify a separator inserted between

Nit: This line was modified to have " each" at the end. If you did that
on the next line, then this diff could have been a touch smaller.

> -   trailer lines. When this option is not given each trailer key-value
> -   pair is separated by ": ". Otherwise it shares the same semantics
> +** 'key_value_separator=<sep>': specify a separator inserted between each
> +   trailer's key and value. When this option is not given each trailer
> +   key-value pair is separated by ": ". Otherwise it shares the same semantics
>     as 'separator=<sep>' above.

LGTM.

It's probably not worth re-rolling, but a small suggestion I have is to
simplify the language a bit to reduce repetition, like so:

    ** 'key_value_separator=<sep>': specify the separator between
       the key and value of each trailer. Defaults to ": ". Otherwise it
       shares the same semantics as 'separator=<sep>' above.

Thanks.