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

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 9, 2021, 19:02 UTC
Message-ID: <CAP8UFD0q=7JT1Wsbr6wj9_LEh41ZKbvKsgnE+_3hSM3aMnjwzw@mail.gmail.com>
In-Reply-To: <505903811df83cf26f4dd70c5b811dde169896a2.1617975462.git.gitgitgadget@gmail.com>

On Fri, Apr 9, 2021 at 3:37 PM ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 9 quoted lines

>
> From: ZheNing Hu <adlternative@gmail.com>
>
> In the original documentation of `trailer.<token>.command`,
> some descriptions are easily misunderstood. So let's modify
> it to increase its readability.
>
> In addition, clarify that `$ARG` in command can only be
> replaced once since `$ARG` is text replacement.

I think you can remove the "since `$ARG` is text replacement" part. Otherwise this looks fine.

Show 11 quoted lines

> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>  Documentation/git-interpret-trailers.txt | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
> index 96ec6499f001..3e5aa3a65ae9 100644
> --- a/Documentation/git-interpret-trailers.txt
> +++ b/Documentation/git-interpret-trailers.txt
> @@ -237,20 +237,20 @@ trailer.<token>.command::
>         specified <token>.

The beginning of the doc for trailer.<token>.command could already be improved. It is:

       This option can be used to specify a shell command that will
       be called to automatically add or modify a trailer with the
       specified <token>.

Instead we could say for example:

       This option can be used to specify a shell command that will
       be called:
         - once to automatically add a trailer with the specified
<token>, and then
         - each time a '--trailer <token>=<value>' argument to modify
the <value> of the trailer that this option would produce

>  When this option is specified, the behavior is as if a special

I would rather say:

When the specified command is first called to add a trailer with the specified <token>, the behavior is as if a special

Show 7 quoted lines

> -'<token>=<value>' argument were added at the beginning of the command
> -line, where <value> is taken to be the standard output of the
> -specified command with any leading and trailing whitespace trimmed
> -off.
> +'--trailer <token>=<value>' argument was added at the beginning of
> +the "git interpret-trailers" command, where <value> is taken to be the
> +standard output of the specified command with any leading and trailing

Here we can remove "specified" as we now use it at the beginning of the sentence.

Show 8 quoted lines

> +whitespace trimmed off.
>  +
> -If the command contains the `$ARG` string, this string will be
> -replaced with the <value> part of an existing trailer with the same
> -<token>, if any, before the command is launched.
> +The first occurrence of substring `$ARG` will be replaced with the
> +<value> part of an existing trailer with the same <token>, if any,
> +before the command is launched.

I think we should not talk about `$ARG` at this point, let's remove this and see what we can do below in the next paragraph or after it.

>  If some '<token>=<value>' arguments are also passed on the command

s/'<token>=<value>'/'--trailer <token>=<value>'/

Show 5 quoted lines

> -line, when a 'trailer.<token>.command' is configured, the command will
> -also be executed for each of these arguments. And the <value> part of
> -these arguments, if any, will be used to replace the `$ARG` string in
> -the command.
> +line, when a 'trailer.<token>.command' is configured, the command is run

s/when a 'trailer.<token>.command' is configured, // s/the command is run/the command is called again/

> +once for each these arguments with the same <token>.

s/each these/each of these/

> And the <value>
> +part of these arguments, if any, will be used to replace the first `$ARG`
> +string in the command.

s/first `$ARG` string/first occurrence of substring `$ARG`/

Let's also add something to explain the purpose of this, for example we could add:

"This way the command can produce a <value> computed from the <value> passed in the '--trailer <token>=<value>' argument."

And then let's also explain what happens when the command is called the first time, with for example:

"For consistency, the first occurrence of substring `$ARG` is also replaced, this time with the empty string, in the command when the command is first called to add a trailer with the specified <token>."

To sum up this would give the following (not properly formatted) description:

-------------------
trailer.<token>.command::
       This option can be used to specify a shell command that will be called:
         - once to automatically add a trailer with the specified
<token>, and then
         - each time a '--trailer <token>=<value>' argument to modify
the <value> of the trailer that this option would produce

       When the specified command is first called to add a trailer
with the specified <token>, the behavior is as if a special
       '--trailer <token>=<value>' argument was added at the beginning
of the "git interpret-trailers" command, where <value>
       is taken to be the standard output of the command with any
leading and trailing whitespace trimmed off.

       If some '--trailer <token>=<value>' arguments are also passed
on the command
       line, the command is called again once for each of these
arguments with the same <token>.
       And the <value> part of these arguments, if any, will be used
to replace the first occurrence
       of substring `$ARG` in the command. This way the command can
produce a <value>
       computed from the <value> passed in the '--trailer
<token>=<value>' argument.

       For consistency, the first occurrence of substring `$ARG` is
also replaced, this time with the
       empty string, in the command when the command is first called
to add a trailer with the
       specified <token>.
-------------------

Thanks!