All of lore.kernel.org
 help / color / mirror / Atom feed
From: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Christian Couder <christian.couder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	ZheNing Hu <adlternative@gmail.com>
Subject: [PATCH v10 0/2] [GSOC] trailer: add new .cmd config option
Date: Fri, 16 Apr 2021 08:47:53 +0000	[thread overview]
Message-ID: <pull.913.v10.git.1618562875.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.913.v9.git.1618245568.gitgitgadget@gmail.com>

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@pobox.com>
          Helped-by: Christian Couder <christian.couder@gmail.com>
     @@ 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

  parent reply	other threads:[~2021-04-16  8:48 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 14:53 [PATCH] [GSOC]trailer: change $ARG to environment variable ZheNing Hu via GitGitGadget
2021-03-24 15:42 ` [PATCH v2] [GSOC]trailer: pass arg as positional parameter ZheNing Hu via GitGitGadget
2021-03-24 20:18   ` Junio C Hamano
2021-03-25  1:43     ` ZheNing Hu
2021-03-25 11:53   ` [PATCH v3] " ZheNing Hu via GitGitGadget
2021-03-25 22:28     ` Junio C Hamano
2021-03-26 13:29       ` ZheNing Hu
2021-03-26 16:13     ` [PATCH v4] " ZheNing Hu via GitGitGadget
2021-03-27 18:04       ` Junio C Hamano
2021-03-27 19:53         ` Christian Couder
2021-03-28 10:46           ` ZheNing Hu
2021-03-29  9:04             ` Christian Couder
2021-03-29 13:43               ` ZheNing Hu
2021-03-30  8:45                 ` Christian Couder
2021-03-30 11:22                   ` ZheNing Hu
2021-03-30 15:07                     ` ZheNing Hu
2021-03-30 17:14                       ` Junio C Hamano
2021-03-31  5:14                         ` ZheNing Hu
2021-03-31 18:19                           ` Junio C Hamano
2021-03-31 18:29                             ` Junio C Hamano
2021-04-01  3:56                               ` ZheNing Hu
2021-04-01 19:49                                 ` Junio C Hamano
2021-04-02  2:08                                   ` ZheNing Hu
2021-04-01  3:39                             ` ZheNing Hu
2021-03-31 10:05       ` [PATCH v5 0/2] " ZheNing Hu via GitGitGadget
2021-03-31 10:05         ` [PATCH v5 1/2] [GSOC] run-command: add shell_no_implicit_args option ZheNing Hu via GitGitGadget
2021-04-01  7:22           ` Christian Couder
2021-04-01  9:58             ` ZheNing Hu
2021-03-31 10:05         ` [PATCH v5 2/2] [GSOC]trailer: pass arg as positional parameter ZheNing Hu via GitGitGadget
2021-04-01  7:28         ` [PATCH v5 0/2] " Christian Couder
2021-04-01 10:02           ` ZheNing Hu
2021-04-02 13:26         ` [PATCH v6] [GSOC] trailer: add new trailer.<token>.cmd config option ZheNing Hu via GitGitGadget
2021-04-02 20:48           ` Junio C Hamano
2021-04-03  5:08             ` ZheNing Hu
2021-04-04  5:34               ` Junio C Hamano
2021-04-03  5:51             ` Christian Couder
2021-04-04 23:26               ` Junio C Hamano
2021-04-06  3:47                 ` Christian Couder
2021-04-06  3:52                   ` Christian Couder
2021-04-06  5:16                     ` ZheNing Hu
2021-04-06  5:34                       ` Junio C Hamano
2021-04-06  5:37                       ` Junio C Hamano
2021-04-04  5:43             ` ZheNing Hu
2021-04-04  8:52               ` Christian Couder
2021-04-04  9:53                 ` ZheNing Hu
2021-04-02 23:44           ` Junio C Hamano
2021-04-03  3:22             ` ZheNing Hu
2021-04-03  4:31               ` Junio C Hamano
2021-04-03  5:15                 ` ZheNing Hu
2021-04-04 13:11           ` [PATCH v7] " ZheNing Hu via GitGitGadget
2021-04-06 16:23             ` Christian Couder
2021-04-07  4:51               ` ZheNing Hu
2021-04-09 13:37             ` [PATCH v8 0/2] [GSOC] trailer: add new .cmd " ZheNing Hu via GitGitGadget
2021-04-09 13:37               ` [PATCH v8 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-09 19:02                 ` Christian Couder
2021-04-10 13:40                   ` ZheNing Hu
2021-04-09 13:37               ` [PATCH v8 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-09 20:18                 ` Christian Couder
2021-04-10 14:09                   ` ZheNing Hu
2021-04-09 19:59               ` [PATCH v8 0/2] " Christian Couder
2021-04-12 16:39               ` [PATCH v9 " ZheNing Hu via GitGitGadget
2021-04-12 16:39                 ` [PATCH v9 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-12 20:42                   ` Junio C Hamano
2021-04-16 12:03                     ` Christian Couder
2021-04-17  1:54                       ` Junio C Hamano
2021-04-12 16:39                 ` [PATCH v9 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-12 20:51                   ` Junio C Hamano
2021-04-13  7:33                     ` Christian Couder
2021-04-13 12:02                       ` ZheNing Hu
2021-04-13 19:18                         ` Junio C Hamano
2021-04-14 13:27                           ` ZheNing Hu
2021-04-14 20:33                             ` Junio C Hamano
2021-04-15 15:32                               ` ZheNing Hu
2021-04-15 17:41                                 ` Junio C Hamano
2021-04-16 12:54                               ` Christian Couder
2021-04-13 18:14                       ` Junio C Hamano
2021-04-16  8:47                 ` ZheNing Hu via GitGitGadget [this message]
2021-04-16  8:47                   ` [PATCH v10 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-16 19:11                     ` Junio C Hamano
2021-04-16  8:47                   ` [PATCH v10 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-16 19:13                     ` Junio C Hamano
2021-04-16 19:21                     ` Junio C Hamano
2021-04-16 19:25                       ` Junio C Hamano
2021-04-17  2:58                         ` Junio C Hamano
2021-04-17  3:36                           ` Junio C Hamano
2021-04-17  7:41                             ` ZheNing Hu
2021-04-17  8:11                               ` Junio C Hamano
2021-04-17 15:13                   ` [PATCH v11 0/2] " ZheNing Hu via GitGitGadget
2021-04-17 15:13                     ` [PATCH v11 1/2] [GSOC] docs: correct description of .command ZheNing Hu via GitGitGadget
2021-04-17 15:13                     ` [PATCH v11 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-17 22:26                     ` [PATCH v11 0/2] " Junio C Hamano
2021-04-18  7:47                       ` ZheNing Hu
2021-04-21  0:09                         ` Junio C Hamano
2021-04-21  5:47                           ` ZheNing Hu
2021-04-21 23:40                             ` Junio C Hamano
2021-04-22  9:20                               ` ZheNing Hu
2021-04-27  6:49                                 ` Junio C Hamano
2021-04-27 12:24                                   ` ZheNing Hu
2021-05-03 15:41                     ` [PATCH v12 " ZheNing Hu via GitGitGadget
2021-05-03 15:41                       ` [PATCH v12 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-05-03 15:41                       ` [PATCH v12 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.913.v10.git.1618562875.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=adlternative@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.