Re: [PATCH v8 0/2] [GSOC] trailer: add new .cmd config option
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Apr 9, 2021, 19:59 UTC
- Message-ID
- <CAP8UFD3kyFe9KdXEu4R1prasnTVAoEW6YOQeA1YDm0k78M3zRQ@mail.gmail.com>
- In-Reply-To
- <pull.913.v8.git.1617975462.gitgitgadget@gmail.com>
On Fri, Apr 9, 2021 at 3:37 PM ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 7 quoted lines
> > In https://lore.kernel.org/git/xmqqv99i4ck2.fsf@gitster.g/ Junio and > Christian talked about the problem of using strbuf_replace() to replace > $ARG: > > 1. if the user's script has more than one $ARG, only the first one will be > replaced, which is incorrected.
Maybe: s/is incorrected/can be puzzling/
> 2. $ARG is textually replaced without shell syntax, which may result a
Not sure what "without shell syntax" means here.
Also: s/may result a/may result in a/
> broken command when $ARG include some unmatching single quote, very
s/when/if/ s/include/includes/ s/some unmatching/an unmatched/
> unsafe. > > Now pass trailer value as $1 to the trailer command with another > trailer.<token>.cmd config, to solve these above two problems,
I think the important thing here is to explain that we want to introduce a new 'trailer.<token>.cmd' config option, so that we can start deprecating 'trailer.<token>.command' when people have started using the new 'trailer.<token>.cmd' config option. Maybe something like:
"To address these issues, let's introduce a new 'trailer.<token>.cmd' config option that behaves in the same way as 'trailer.<token>.command' except that it passes the trailer value as $1 to the configured command instead of textually replacing the first occurence of '$ARG' in it. This will let us slowly deprecate 'trailer.<token>.command' in favor of 'trailer.<token>.cmd' in the future."
> We are now writing documents that are more readable and correct than before.
I would suggest removing this sentence as I don't think it adds much to the above.
> ZheNing Hu (2): > [GSOC] docs: correct descript of trailer.<token>.command
By the way the following title might be a bit better and shorter:
"[GSOC] docs: improve 'trailer.<token>.command' doc"
> [GSOC] trailer: add new .cmd config option
Maybe we can afford: s/.cmd/'trailer.<token>.cmd'/
> - [GSOC] trailer: add new trailer.<token>.cmd config option > + [GSOC] trailer: add new .cmd config option
Was the previous title too long? Or is there an issue with GitGitGadget because it contains <token>?