Subject: Re: [PATCH v9 2/2] [GSOC] trailer: add new .cmd config option

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Apr 2021 11:14:32 -0700

Christian Couder <[email-masked]> writes:

> It might be desirable to make it easier for people to migrate from
> ".command" to ".cmd". I agree this is debatable, but I don't see a big
> downside in it. Maybe, if no argument was passed at all the first time
> the command is called instead of the empty string, the command could
> then know that it's called for the first time. I am not sure this
> would be very helpful in practice though.

An integration of the trailer mechanism into the "git commit"
command, which cannot emulate "-s" at the end-user level, is
something I would call a big failure, though.

>> This and the other (omitted) example demonstrates how the initial
>> "empty" invocation is useless by using "replace".  Which also means
>> that you cannot add more than one trailer of the same <key> with the
>> mechanism (since the older ones are replaced with the latest).
>
> You can add more than one trailer with the same key if you don't use
> "replace" but use "--trim-empty" on the command line, so that an empty
> trailer added by the initial invocation is removed. And we can later
> add a `trailer.<token>.runMode` to remove the initial invocation.

As I said in the other message (not the one you are responding to),
trim-empty is not a viable workaround.  Imagine what happens when
you want to always add a trailer (unrelated to signed-off-by), for
which it is perfectly valid not to have any value.  An attempt to
work around the problem "extra call without being asked from the
command line" behaviour has on our emulated "signed-off-by" example
with --trim-empty would nuke such an unrelated trailer.

Besides, if the command produced non-empty value, e.g.

	[trailer "baz"] command = "echo B $ARG Z"

the extra call without being asked from the command line will still
give "baz: B  Z" that is not empty.

So... I am very curious to learn what the intended use case is for
this puzzling feature.  If it turns out to be that it behaves only
because it was coded that way without any real benefit, I would
strongly prefer not to carry it over to the new .cmd thing, which is
an attempt to deprecate .command to get rid of its broken parts (so
far, we identified two---only the first occurrence is replaced, and
the replacement can break command line syntax) while salvaging its
good parts (being able to programmatically decide the value).  And
so far what I heard hasn't convinced me that this behaviour falls
into the latter category.

Thanks.