Re: [PATCH v13] [GSOC] commit: add --trailer option

From: ZheNing Hu <adlternative@gmail.com>
Date: Mar 23, 2021, 06:11 UTC
Message-ID: <CAOLTT8Rc6Y4=a-2aj96hs24-h4zywxO0nLxBwh_B19hSSxcs_w@mail.gmail.com>
In-Reply-To: <CAP8UFD3fYTc8=y+kru-mN5KmTsnqc6X8mf14VtyWf1Nj9CJ1EQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> 于2021年3月23日周二 上午5:34写道：

Show 38 quoted lines

>
> On Mon, Mar 22, 2021 at 11:23 AM ZheNing Hu <adlternative@gmail.com> wrote:
> >
> > Christian Couder <christian.couder@gmail.com> 于2021年3月22日周一 下午3:43写道：
>
> > > Nice that you have added such a test!
> >
> > Thanks.
> >
> > But at the same time I have two little doubt.
> >
> > 1.
> > If we have your config:
> >
> > $ git config trailer.sign.key "Signed-off-by: "
> > $ git config trailer.sign.ifexists replace
> > $ git config trailer.sign.command "git log --author='\$ARG' -1
> > --format='format:%aN <%aE>'"
> >
> > Then I touch a test.c and use:
> >
> > $ git interpret-trailers --in-place test.c
> >
> > without `--trailer`, See what is happen:
> >
> > It seem like your local repo last commit "name <email>" pair
> > have been record in `test.c`.
> >
> > Could this be considered a bug?
>
> First it seems strange to use `git interpret-trailers` on a "test.c"
> file. It's supposed to be used on commit messages.
>
> Then, as the doc says, every command specified by any
> "trailer.<token>.command" config option is run at least once when `git
> interpret-trailers` is run. This is because users might want to
> automatically add some trailers all the time.
>

Well, I understand it now.

Show 9 quoted lines

> If you want nothing to happen when $ARG isn't set, you can change the
> config option to something like:
>
> $ git config trailer.sign.command "NAME='\$ARG'; test -n \"\$NAME\" &&
> git log --author=\"\$NAME\" -1 --format='format:%aN <%aE>' || true"
>
> (This is because it looks like $ARG is replaced only once with the
> actual value, which is perhaps a bug. Otherwise something like the
> following might work:

this is because `$ARG` is replaced in "trailer.c" by "strbuf_replace" which only replcae the specified string for only one time, I think `strbuf_replace()` can be changed like:

@@ -110,8 +110,9 @@ static inline int is_blank_line(const char *str)

 static inline void strbuf_replace(struct strbuf *sb, const char *a,
const char *b)
 {
-       const char *ptr = strstr(sb->buf, a);
-       if (ptr)
+       char *ptr = sb->buf;
+
+       while ((ptr = strstr(ptr, a)))
                strbuf_splice(sb, ptr - sb->buf, strlen(a), b, strlen(b));
 }

>
> git config trailer.sign.command "test -n '\$ARG' && git log
> --author='\$ARG' -1 --format='format:%aN <%aE>' || true")
>
> Then you can run `git interpret-trailers` with the --trim-empty option
> like this:
>
> ------
> $ git interpret-trailers --trim-empty --trailer sign=Linus<<EOF
> EOF
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ------
>
> or like:
>
> ------
> $ git interpret-trailers --trim-empty<<EOF
> > EOF
>
> ------

Thanks for effective solution.
Maybe I should also correct this part of the test.