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

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 16, 2021, 12:54 UTC
Message-ID: <CAP8UFD2J8jz2Ku4ur_L+36b9hKB5drrexKNHYF6dM9G9BKhFug@mail.gmail.com>
In-Reply-To: <xmqqv98oy5bh.fsf@gitster.g>

On Wed, Apr 14, 2021 at 10:33 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 27 quoted lines

>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> >> So I am waiting to hear why it is not a misfeature.  If it is not,
> >> then surely I am fine to keep it for now and add a workaround later,
> >> but until that happens, I do not think "commit --trailer" can be
> >> used as a way to allow end-users emulate "-s" for their favorite
> >> trailer like helped-by, acked-by, etc.
> >>
> >
> > If it is really necessary to solve this "empty execution" in .cmd,
>
> > Maybe we need to consider two points:
> > * Do we need a new config flag as you said `[implicitExecution = false]`
> > or just drop it? Has anyone been relying on the "empty execution" of
> > .command before? This may be worthy of concern.
>
> Yes, if it is useful sometimes to run the .command or .cmd with
> empty <value> even when nobody asks for it on the command line with
> a "--trailer=<key>:<value>" option, then I agree that the users
> should be able to choose between running and not running [*].
>
> > *  Do we need `trailer.<token>.runMode` as Christan said before?
> > I rejected his this suggestion before, and now I regret it a bit.
>
> So far, I haven't heard anything that indicates it is a useful
> behaviour for .command,

Well the `git config trailer.sign.command 'echo "$(git config user.name) <$(git config user.email)>"'` has been documented in the EXAMPLES section of the `git interpret-trailers` doc since when ".command" was implemented, and I believe that reviewers thought that it was a good feature to have then.

Show 12 quoted lines

> so my preference is to get rid of the
> behaviour when we introduce .cmd to deprecate .command; yes, until
> we get rid of .command, the behaviour between the two would be
> inconsistent but that is unavoidable when making a bugfix that is
> backward incompatible.
>
> When (and only when) anybody finds a credible use case, we can add a
> mechanism to optionally turn it on (e.g. .runMode).
>
> That is my thinking right at this moment, but that's of course
> subject to change when a use case that would be helped by having
> this extra execution.

Such use cases and some of this were discussed when `git interpret-trailers` was initially implemented.

For example in https://lore.kernel.org/git/CAP8UFD2oXpW9QEkSh+vpNGRAxRFp0zJF39ZZ8sUZLTcKB9mHWQ@mail.gmail.com/ I suggested the following:

         [trailer "m-o-b"]
                 key = "Made-on-branch: "
                 command = "git name-rev --name-only HEAD"

when someone wanted a way to always add "a trailer for the branch that the commit was checked in to at the time"

In https://lore.kernel.org/git/534414FB.6040604@alum.mit.edu/ Michael Haggerty suggested:

"Maybe there should be a trailer.<token>.trimEmpty config option."

I haven't fully checked the discussions, so there might be other examples.