[BUG?] trailer command with multiple keys

[BUG?] trailer command with multiple keys

[Michael J Gruber]

The command

printf "body\n\ntest: foo\ntest: froz\n" | git -c
trailer.test.key=tested -c trailer.test.command="echo by \$ARG"
interpret-trailers

gives:

body

tested: foo
tested: froz
tested: by froz

I expected the command to be run on each "test" key, resulting in the
output:

body:

tested: by foo
tested: by froz

(In a real life scenario, I would use ifexists replace.)[*]

Maybe my expectation is wrong? The code breaks out of the loop after the
first matching in_tok, apparently intentionally so. But I'm not sure -
the key is replaced for both instances.

Simply replacing that "return 1" by a "ret = 1" etc. runs into problems
with the way the freeing of in_tok and arg_tok is arranged there :|

Basically, I expected the trailer command to work "grep/sed-like" on all
key value pairs that have matching keys, passing the value to the
command, and using the (each) command's output as the new value for each
of these pairs.

Michael

[*] My prime use case: fill in reported-by etc. with short author names,
completed the same way we complete --author=jun using a trailer command
(interpret-trailers in the commit-msg hook):

$ git help author
`git author' is aliased to `!f() { a=$(git log -1 --all -i --format="%aN
<%aE>" --author "$1"); echo ${a:-$1}; }; f'

$ cat .git/hooks/commit-msg
#!/bin/sh
git interpret-trailers --in-place "$1"

$ git config --get-regexp trailer
trailer.report.key Reported-by
trailer.report.command git author '$ARG'
trailer.report.ifexists replace
trailer.report.ifmissing doNothing

Re: [BUG?] trailer command with multiple keys

[Christian Couder]

On Mon, Jun 6, 2016 at 2:27 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> The command
>
> printf "body\n\ntest: foo\ntest: froz\n" | git -c
> trailer.test.key=tested -c trailer.test.command="echo by \$ARG"
> interpret-trailers
>
> gives:
>
> body
>
> tested: foo
> tested: froz
> tested: by froz
>
> I expected the command to be run on each "test" key, resulting in the
> output:
>
> body:
>
> tested: by foo
> tested: by froz
>
> (In a real life scenario, I would use ifexists replace.)[*]
>
> Maybe my expectation is wrong?

I wouldn't say that it is wrong, but for now trailer configuration
applies mostly to trailer passed as argument to `git
interpret-trailers`.

So you could perhaps get something close to what you want with:

--------
$ printf "body\n\n" | git -c trailer.test.key=tested -c
trailer.test.command="/home/christian/git/test/interpret-trailers/trailer-script.sh
\$ARG"  interpret-trailers --trim-empty --trailer='test: foo'
--trailer='test: frotz'
body

tested: by foo
tested: by frotz
--------

and:

--------
$ cat /home/christian/git/test/interpret-trailers/trailer-script.sh
#!/bin/sh
test -n "$1" && echo "by $1"
:
--------

> The code breaks out of the loop after the
> first matching in_tok, apparently intentionally so. But I'm not sure -
> the key is replaced for both instances.
>
> Simply replacing that "return 1" by a "ret = 1" etc. runs into problems
> with the way the freeing of in_tok and arg_tok is arranged there :|
>
> Basically, I expected the trailer command to work "grep/sed-like" on all
> key value pairs that have matching keys, passing the value to the
> command, and using the (each) command's output as the new value for each
> of these pairs.

Yeah, it could have been designed like that.

The problem is that something like:

               $ git config trailer.sign.key "Signed-off-by: "
               $ git config trailer.sign.ifmissing add
               $ git config trailer.sign.ifexists doNothing
               $ git config trailer.sign.command 'echo "$(git config
user.name) <$(git config user.email)>"'

and piping any commit message into "git interpret-trailers" should work too.

In short it's not very natural to have both of the following by default:

- a configured command should run once to get a chance to add a new
trailer, even it doesn't exist in the input file
- a configured command should run once per trailer in the input file

(especially because as I said above for now configuration applies
mostly to trailers on the command line).

One solution could be to add support for a new
trailer.<token>.commandMode config option that could take values like
"onceAnyway", "oncePerMatchingTrailerInInputFile",
"oncePerMatchingTrailerOnCommandLine" and it should be possible to use
one or more of those modes, like for example:

trailer.stuff.commandMode=onceAnyway,oncePerMatchingTrailerInInputFile

> [*] My prime use case: fill in reported-by etc. with short author names,
> completed the same way we complete --author=jun using a trailer command
> (interpret-trailers in the commit-msg hook):
>
> $ git help author
> `git author' is aliased to `!f() { a=$(git log -1 --all -i --format="%aN
> <%aE>" --author "$1"); echo ${a:-$1}; }; f'
>
> $ cat .git/hooks/commit-msg
> #!/bin/sh
> git interpret-trailers --in-place "$1"
>
> $ git config --get-regexp trailer
> trailer.report.key Reported-by
> trailer.report.command git author '$ARG'
> trailer.report.ifexists replace
> trailer.report.ifmissing doNothing

Yeah, it would be nice to be able to have things like that.