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. 2. $ARG is textually replaced without shell syntax, which may result a broken command when $ARG include some unmatching single quote, very unsafe. At the same time, the trailer.<token>.command has another design disadvantages: It will automatically execute with a empty value in <token> <value> pair, which will create a trailer that we don’t expect. Now pass trailer value as $1 to the trailer command with another trailer.<token>.cmd config, to solve these above problems. We are now writing documents that are more readable and correct than before. ZheNing Hu (2): [GSOC] docs: correct descript of trailer.<token>.command [GSOC] trailer: add new .cmd config option Documentation/git-interpret-trailers.txt | 93 ++++++++++++++++++++---- t/t7513-interpret-trailers.sh | 84 +++++++++++++++++++++ trailer.c | 35 ++++++--- 3 files changed, 186 insertions(+), 26 deletions(-) base-commit: 142430338477d9d1bb25be66267225fb58498d92 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-913%2Fadlternative%2Ftrailer-pass-ARG-env-v10 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-913/adlternative/trailer-pass-ARG-env-v10 Pull-Request: https://github.com/gitgitgadget/git/pull/913 Range-diff vs v9: 1: 8129ef6c476b = 1: 8129ef6c476b [GSOC] docs: correct descript of trailer.<token>.command 2: 7f645ec95f48 ! 2: daa889bd0ade [GSOC] trailer: add new .cmd config option @@ Commit message replaced with the value given to the `interpret-trailer` command for the token in a '--trailer <token>=<value>' argument. - This has two downsides: + This has three downsides: * The use of $ARG in the mechanism misleads the users that the value is passed in the shell variable, and tempt them @@ Commit message a broken command that is not syntactically correct (or worse). + * The first occurrence of substring `$ARG` will be replaced + with the empty string, in the command when the command is + first called to add a trailer with the specified <token>. + This is a bad design, the nature of automatic execution + causes it to add a trailer that we don't expect. + Introduce a new `trailer.<token>.cmd` configuration that takes higher precedence to deprecate and eventually remove `trailer.<token>.command`, which passes the value as an argument to the command. Instead of "$ARG", users can refer to the value as positional argument, $1, in their - scripts. + scripts. At the same time, in order to allow + `git interpret-trailers` to better simulate the behavior + of `git command -s`, 'trailer.<token>.cmd' will not + automatically execute. Helped-by: Junio C Hamano <gitster@xxxxxxxxx> Helped-by: Christian Couder <christian.couder@xxxxxxxxx> @@ Documentation/git-interpret-trailers.txt: leading and trailing whitespace trimme -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. -+of these arguments, if any, will be passed to the command as its -+first argument. 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>. -+For consistency, the $1 is also passed, this time with the empty string, -+in the command when the command is first called to add a trailer with -+the specified <token>. ++of these arguments, if any, will be passed to the command as its ++first argument. This way the command can produce a <value> computed ++from the <value> passed in the '--trailer <token>=<value>' argument. EXAMPLES -------- @@ Documentation/git-interpret-trailers.txt: subject +#!/bin/sh +test -n "$1" && git shortlog -s --author="$1" HEAD || true +$ git config trailer.cnt.key "Commit-count: " -+$ git config trailer.cnt.ifExists "replace" ++$ git config trailer.cnt.ifExists "addIfDifferentNeighbor" +$ git config trailer.cnt.cmd "~/bin/gcount" -+$ git interpret-trailers --trailer="cnt:Junio" <<EOF ++$ git interpret-trailers --trailer="cnt:Junio" --trailer="cnt:Linus Torvalds"<<EOF +> subject +> +> message @@ Documentation/git-interpret-trailers.txt: subject +message + +Commit-count: 22484 Junio C Hamano ++Commit-count: 1117 Linus Torvalds +------------ + +* Configure a 'ref' trailer with a cmd use a global script `glog-grep` @@ t/t7513-interpret-trailers.sh: test_expect_success 'setup' ' + git config trailer.bug.cmd "echo \"maybe is\"" && + cat >expected2 <<-EOF && + -+ Bug-maker: maybe is + Bug-maker: maybe is him + Bug-maker: maybe is me + EOF @@ t/t7513-interpret-trailers.sh: test_expect_success 'setup' ' +test_expect_success 'with cmd and $1' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && -+ git config trailer.bug.ifExists "replace" && ++ git config trailer.bug.ifExists "add" && + git config trailer.bug.cmd "echo \"\$1\" is" && + cat >expected2 <<-EOF && + ++ Bug-maker: him is him + Bug-maker: me is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ @@ trailer.c: static int git_trailer_config(const char *conf_key, const char *value case TRAILER_WHERE: if (trailer_set_where(&conf->where, value)) warning(_("unknown value '%s' for key '%s'"), value, conf_key); -@@ trailer.c: static void process_command_line_args(struct list_head *arg_head, - /* Add an arg item for each configured trailer with a command */ - list_for_each(pos, &conf_head) { - item = list_entry(pos, struct arg_item, list); -- if (item->conf.command) -+ if (item->conf.cmd || item->conf.command) - add_arg_item(arg_head, - xstrdup(token_from_item(item, NULL)), - xstrdup(""), -- gitgitgadget
