Re: [PATCH v9 1/2] [GSOC] docs: correct descript of trailer.<token>.command

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 17, 2021, 01:54 UTC
Message-ID: <xmqqtuo5iski.fsf@gitster.g>
In-Reply-To: <CAP8UFD2gUbWP2LWXDt1nmph1qMyhEBfLErw3=5OTBx-KeBQBVQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 7 quoted lines

> $ git config trailer.sign.command 'echo "$(git config user.name)
> <$(git config user.email)>"'
>
> My opinion is that we should have added a `trailer.<token>.runMode`
> config option along with `trailer.<token>.command`. This was not
> discussed unfortunately when ".command" was implemented, but it seems
> to be a good idea now.

Yes, without a knob to diable/enable, the "feature" is pretty much useless.

> It can be avoided when the --trim-empty CLI option can be used. A hook
> to remove empty trailers (which might call `git interpret-trailers
> --trim-empty` itself) could also be used when --trim-empty cannot be
> used directly.

And as you know, --trim-empty that applies to all trailer keys would destroy other trailers and trailer.<token>.trimEmpty, even if it were available, would not work at all to remove it for the case you cited above, to add "user.name <user.email>", which is not an empty string.

> I agree that the current mechanism cannot easily emulate "commit -s".
> ...
> I agree that --trim-empty is not usable sometimes. Another idea would
> be to add 'trailer.<token>.trimEmpty' to be able to do things like:

"easily"?  "sometimes"?

"--trim-empty" is unusable, trailer.<token>.trimEmpty would not work even if it were added because the reason why this is broken is not because it gives an empty value, but because it runs even when it is not asked and there is no way to turn it off.

This shows that even the only plausibly-useful use case we've seen so far is not very well supported, and necessary options/knobs to make it usable have not been invented and implemented.

It is time for us to admit that this is a misfeature that is not well thought out. Recognising that .command is broken is the first step to remedy it with a better alternative in .cmd; otherwise we would inherit the same breakage in the replacement.

The only reasonable way out I would think of is to hide the unconditional execution behind trailer.<token>.runMode, and make the default for runMode to "do not run when --trailer=<token>[:<value>] is not asked from the command line" for .cmd; it is OK for .command not to honor the knob, as we will get rid of it once we see .cmd works well.

Thanks.