Re: [PATCH v3 2/5] pretty: allow showing specific trailers

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2018, 07:52 UTC
Message-ID: <xmqq4lc4wa0o.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <878t1g72ee.fsf@0x63.nu>

Anders Waldenborg <anders@0x63.nu> writes:

> Would it feel less inconsistent if it did not set the 'only_trailers'
> option?

If %(trailers:key=...) did not automatically imply 'only', it would be very consistent.

But as I already said, I think it would be less convenient, as I do suspect that those who want specific keys would want to see only those trailers with specific keys.

And if we want that convinience, we'd probably want a way to optionally disable that 'only' bit when the user wants to.

And...

Show 12 quoted lines

> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -228,9 +228,9 @@ endif::git-rev-list[]
>  ** '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. Non-trailer lines in the trailer block are also hidden
> -   (i.e. 'key' implies 'only'). E.g., `%(trailers:key=Reviewed-by)`
> -   shows trailer lines with key `Reviewed-by`.
> +   shown. Non-trailer lines in the trailer block are also hidden.
> +   E.g., `%(trailers:key=Reviewed-by)` shows trailer lines with key
> +   `Reviewed-by`.

... I do not think this change reduces the puzzlement felt by readers who would have wondered how that implied 'only' can be countermanded with the old text. It just makes it look even less explained to them.

If we assume that nobody would ever want to mix non-trailers when asking specific keywords, then "them" in the above paragraph would become an empty set, and we do not have to worry about them. I am not sure if Git is still such a small project to allow us rely on such an assumption, though.

>  ** 'only': omit non-trailer lines from the trailer block.
>  ** 'unfold': make it behave as if interpret-trailer's `--unfold`
>     option was given. E.g., `%(trailers:only,unfold)` unfolds and