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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 17, 2021, 02:58 UTC
Message-ID: <xmqqpmytiplw.fsf@gitster.g>
In-Reply-To: <xmqqr1jakp57.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 37 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> OK.  I think there still will be disagreement on this last point
>> between Christian and I, but I'd be happy with this as the first cut
>> for newly introduced .cmd and then when it becomes needed add
>> something like the attached patch on top to optionally run the given
>> command when configured.
>>
>>
>>  trailer.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git c/trailer.c w/trailer.c
>> index 7c7cb61a94..39132211cc 100644
>> --- c/trailer.c
>> +++ w/trailer.c
>> @@ -723,7 +723,8 @@ static void process_command_line_args(struct list_head *arg_head,
>>  	/* Add an arg item for each configured trailer with a command */
>>  	list_for_each(pos, &conf_head) {
>>  		item = list_entry(pos, struct arg_item, list);
>> -		if (item->conf.command)
>> +		if ((item->conf.run_implicitly && item->conf.cmd) ||
>> +		    item->conf.command)
>>  			add_arg_item(arg_head,
>>  				     xstrdup(token_from_item(item, NULL)),
>>  				     xstrdup(""),
>
> Actually, if we were to do this, I actually suspect that, unlike the
> textual replacement of $ARG in the old .command configuration, we
> should take advantage of the fact that the command can tell the
> cases between an empty string given as $1 and no positional argument
> was given for $1.  So what add_arg_item() does here for .command
> (which has to give an empty string, because it will textually
> replace an occurrence of $ARG) and for .cmd may have to be different.
> Perhaps record NULL here, so that when the command line is formed,
> we can tell that we do not want add an extra "" that becomes $1,
> when we are dealing with .cmd in this codepath.

And continuing this line of thought, I think it would be a perfectly fine extension to allow the script/program that is launched by the .command or .cmd mechanism to signal interpret-trailers that it does not want it to add a trailer as the result of this invocation by exiting with non-zero. And that would be a reasonable way forward without having to add yet another ugly workaround .runMode.

For example, trailer.signoff.cmd could be this script:

	#!/bin/sh
	if test $# != 1
	then
		exit 1
	else
		git log -1 --author="$1" --format='"%aN" <$aE>'
	fi

where the "implicit" invocation is signalled by not passing any argument, to which the script reacts by exiting with 1, and the interpret-trailers would discard the result of the unasked-for invocation.