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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 20, 2018, 05:59 UTC
Message-ID: <xmqqy39o9tmq.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181118114427.1397-3-anders@0x63.nu>

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

Show 7 quoted lines

> +  followed by a colon and zero or more comma-separated options:
> +  ** 'only': omit non-trailer lines from the trailer block.
> +  ** 'unfold': make it behave as if interpret-trailer's `--unfold`
> +     option was given.
> +  ** 'key=<K>': only show trailers with specified key. Matching is
> +     done case-insensitively and trailing colon is optional. If option
> +     is given multiple times only last one is used.

It would be good to allow multiple keys, as

	%(trailers:key=signed-off-by,key=helped-by)

and

	%(trailers:key=signed-off-by)%(trailers:key=helped-by)

would mean quite a different thing. The former can preserve the order of these sign-offs and helped-bys in the original, while the latter would have to show all the sign-offs before showing the helped-bys, and I am not convinced if the latter is the only valid use case.

Also, use of 'key=' automatically turns on 'only' as described, and I tend to agree that it would a convenient default mode (i.e. when picking certain trailers only with this mechanism, it is likely that the user is willing to use %(subject) etc. to fill in what was lost by the implicit use of 'only'), but at the same time, it makes me wonder if we need to have a way to countermand an 'only' (or 'unfold' for that matter) that was given earlier, e.g.

	%(trailers:key=signed-off-by,only=no)

Thanks.