Junio C Hamano <gitster@xxxxxxxxx> 于2021年4月3日周六 上午4:49写道：
> > +the behavior is as if a special '<token>=<value>' argument were added at
> > +the beginning of the command, <value> will be passed to the user's
> > +command as an empty value.
>
> Do the two occurrences of the word "command" in the sentence refer
> to different things? I do not think this is an existing problem
> inherited from the original, but as we are trying to improve the
> description, I wonder if we can clarify them a bit.
>
> ... as if a '<token>=<value>' argument were added at the
> beginning of the "git interpret-trailers" command, the
> command specified by this configuration variable will be
> called with an empty string as the argument.
>
> is my attempt, but I am not still sure what that "as if" part is
> trying to say. Does it mean with
>
> [trailer "Foo"] cmd = foo-cmd
>
> and the 'input-file' does not have "Foo: <some existing value>"
> trailer in it, the command "git interpret-trailers input-file"
> would behave as if this command was run
>
> $ Foo= git interpret-trailers input-file
>
> (as there is no <value>, I am not sure what <value> is used when
> <token>=<value> is prefixed to the command)?
>
> Puzzled and confused utterly am I... Help, Christian?
>
If we don’t want a input-file without any trailers to execute this command,
I’m thinking that we maybe need "trailer.<token>.ifmissing=doNothing"
or something else, otherwise execute this 'alwaysRunCmd' for create a
empty trailer like "Signed-off-by" and users can use it to add content.
> > +
> > If some '<token>=<value>' arguments are also passed on the command
> > line, when a 'trailer.<token>.command' is configured, the command will
> > also be executed for each of these arguments. And the <value> part of
>
> This talks about 'trailer.<token>.command'. Should this be changed
> to '.cmd'?
>
> Or does everything after "When this option is specified, if there is
> no trailer with ..." apply to both the old .command and new .cmd?
> If so, that was not clear at all---we'd need to clarify this part.
>
Because ".command" will be eliminated, may be only leave those
description Information to ".cmd" is better.
> > -these arguments, if any, will be used to replace the `$ARG` string in
> > -the command.
> > +these arguments, if any, will be passed to the command as first parameter.
> >
> > EXAMPLES
> > --------
> > @@ -333,6 +346,55 @@ subject
> > Fix #42
> > ------------
> >
> > +* Configure a 'see' trailer with a cmd use a global script `git-one`
> > + to show the subject of a commit that is related, and show how it works:
> > ++
> > +------------
> > +$ cat ~/bin/git-one
> > +#!/bin/sh
> > +git show -s --pretty=reference "$1"
> > +$ git config trailer.see.key "See-also: "
> > +$ git config trailer.see.ifExists "replace"
> > +$ git config trailer.see.ifMissing "doNothing"
> > +$ git config trailer.see.cmd "~/bin/git-one"
> > +$ git interpret-trailers <<EOF
> > +> subject
> > +>
> > +> message
> > +>
> > +> see: HEAD~2
> > +> EOF
> > +subject
> > +
> > +message
> > +
> > +See-also: fe3187e (subject of related commit, 2021-4-2)
> > +------------
> > +
> > +* Configure a 'who' trailer with a cmd use a global script `git-who`
> > + to find the recent matching "author <mail>" pair in git log and
> > + show how it works:
> > ++
> > +------------
> > +$ cat ~/bin/git-who
> > + #!/bin/sh
> > + git log -1 --format="%an <%ae>" --author="$1"
>
> Unusual indentation here. But more importantly, I am not sure if
> having both 'see' and 'help' examples is worth it---they are similar
> enough that the second one does not teach anything new to those who
> studied the first one already, aren't they?
>
Ok, I will think about other examples.
> > +$ git config trailer.help.key "Helped-by: "
> > +$ git config trailer.help.ifExists "replace"
> > +$ git config trailer.help.cmd "~/bin/git-who"
> > +$ git interpret-trailers --trailer="help:gitster@" <<EOF
> > +> subject
> > +>
> > +> message
> > +>
> > +> EOF
> > +subject
> > +
> > +message
> > +
> > +Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
> > +------------
> > +
> > * Configure a 'see' trailer with a command to show the subject of a
> > commit that is related, and show how it works:
> > +
>
>
>
> > diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
> > index 6602790b5f4c..923923e57573 100755
> > --- a/t/t7513-interpret-trailers.sh
> > +++ b/t/t7513-interpret-trailers.sh
> > @@ -51,6 +51,77 @@ test_expect_success 'setup' '
> > EOF
> > '
> >
> > +test_expect_success 'with cmd' '
> > + test_when_finished "git config --unset trailer.bug.key && \
> > + git config --unset trailer.bug.ifExists && \
> > + git config --unset trailer.bug.cmd" &&
>
> It is unwise to use && between these three "git config" invocations,
> I suspect. "git config --unset" exits with non-zero status when you
> attempt to remove with an non-existent key, but you would remove the
> .ifExists and .cmd even if .key is not defined. Perhaps
>
> test_when_finished "git config --unset-all trailer.bug.key
> git config --unset-all trailer.bug.ifExists
> git config --unset-all trailer.bug.cmd" &&
>
> would be more sensible. Or if we just want to remove everything
> under the trailer.bug.* section, this might be even better:
>
> test_when_finished "git config --remove-section trailer.bug" &&
>
> as we can add new trailer.bug.* to the system and use them in this
> test, but removing the entire section would still be a good way to
> clean after ourselves.
>
Good idea, This removing is also more efficient.
> > diff --git a/trailer.c b/trailer.c
> > index be4e9726421c..6aeff6a1bd33 100644
> > --- a/trailer.c
> > +++ b/trailer.c
> > ...
> > +static char *apply_command(struct conf_info *conf, const char *arg)
> > {
> > struct strbuf cmd = STRBUF_INIT;
> > struct strbuf buf = STRBUF_INIT;
> > struct child_process cp = CHILD_PROCESS_INIT;
> > char *result;
> >
> > - strbuf_addstr(&cmd, command);
> > - if (arg)
> > - strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
> > -
> > - strvec_push(&cp.args, cmd.buf);
> > + if (conf->cmd) {
> > + // cp.shell_no_implicit_args = 1;
>
> Do not add new code that is commented out. Besides we do not use // comment.
>
> > + strbuf_addstr(&cmd, conf->cmd);
> > + strvec_push(&cp.args, cmd.buf);
> > + if (arg)
> > + strvec_push(&cp.args, arg);
>
> Thanks.
Thanks.
