Re: git interpret-trailers with multiple keys

[Christian Couder <christian.couder@gmail.com>]

On Wed, Apr 6, 2016 at 3:30 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Apr 06, 2016 at 10:42:42AM -0700, Junio C Hamano wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>
>> > On Wed, Apr 06, 2016 at 06:58:30PM +0200, Matthieu Moy wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >>
>> >> > I have this in .git/config
>> >> >
>> >> > [trailer "r"]
>> >> >         key = Reviewed-by
>> >> >         command = "echo \"Michael S. Tsirkin <mst@redhat.com\""
>> >> > [trailer "s"]
>> >> >         key = Signed-off-by
>> >> >         command = "echo \"Michael S. Tsirkin <mst@redhat.com\""
>> >> >
>> >> > whenever I run git interpret-trailers -t r I see these lines added:
>> >> >
>> >> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com
>> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com
>> >> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com
>> >> >
>> >> > Why is Reviewed-by repeated?  Bug or feature?
>> >>
>> >> The first two lines are added unconditionally:
>> >>
>> >> $ echo | git interpret-trailers
>> >>
>> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com
>> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com
>> >>
>> >> The last line is added because you've asked for it with --trailer r.

Yes, and because the default is to add the trailer at the end.

>> >> I don't think it's currently possible to get the behavior you seem to
>> >> expect, ie. to define trailer tokens fully (key and value) in your
>> >> config file but use them only on request.

Yes, because you could define for example a function like this:

reviewed() {
    git interpret-trailers --trailer 'Reviewed-by: Michael S. Tsirkin
<mst@redhat.com>' --in-place "$@"
}

So it is kind of easy already to make things requestable.

If people really want some configured trailers to be used only on
request, it is possible to add a config option for that.

>> >> (BTW, I think you wanted a closing > at the end)
>> >
>> > Is this worth fixing? It doesn't look like a behaviour anyone
>> > would want...
>>
>> CC'ing Christian who's done the "trailers" thing.
>>
>> Personally, I do not think adding any configured trailers without
>> being asked is a sensible behaviour, but it is likely that people
>> already depend on it, as we seem to see "How do I configure to
>> always add this and that trailer?" from time to time.  I do not
>> think it is unreasonable to disable the "automatically add
>> everything that is configured" when the command line arguments ask
>> for some specific trailer, but I haven't thought deeply about it.
>>
>> An additional (uninformed) observation is that the 'echo' looks like
>> an ugly workaround for the lack of "always use this string as the
>> value" configuration.
>
> Or at least a default.
>
>> Perhaps next to trailer.<token>.command, we
>> would need trailer.<token>.value?

Yeah, that is possible too.
It could be bit redundant if we already have a config option to say if
the trailer has to be requested.