On Fri, Apr 9, 2021 at 3:37 PM ZheNing Hu via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > 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>?
