Re: 'git interpret-trailers' is tripped by comment characters other than '#'

From: Masahiro Yamada <yamada.masahiro@socionext.com>
Date: Jun 17, 2019, 04:32 UTC
Message-ID: <CAK7LNASueRR6narhHqe_D93ewXhTPXokWSCvQRh+Q6=-_AB+3g@mail.gmail.com>
In-Reply-To: <20190614150758.GA22984@sigill.intra.peff.net>

On Sat, Jun 15, 2019 at 12:08 AM Jeff King <peff@peff.net> wrote:

Show 31 quoted lines

>
> On Fri, Jun 14, 2019 at 08:35:04PM +0900, Masahiro Yamada wrote:
>
> > Perhaps, 'git interpret-trailers' should be changed
> > to recognize core.commentChar ?
>
> It looks like the trailer code does respect it, but the
> interpret-trailers program never loads the config. Does the patch below
> make your problem go away?
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 8ae40dec47..f101d092b8 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -10,6 +10,7 @@
>  #include "parse-options.h"
>  #include "string-list.h"
>  #include "trailer.h"
> +#include "config.h"
>
>  static const char * const git_interpret_trailers_usage[] = {
>         N_("git interpret-trailers [--in-place] [--trim-empty] [(--trailer <token>[(=|:)<value>])...] [<file>...]"),
> @@ -112,6 +113,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
>                 OPT_END()
>         };
>
> +       git_config(git_default_config, NULL);
> +
>         argc = parse_options(argc, argv, prefix, options,
>                              git_interpret_trailers_usage, 0);
>

Yup, works for me.

Show 6 quoted lines

> I do wonder if the trailer code is correct to always respect it, though.
> For example, in "git log" output we'd expect to see commit messages from
> people with all sorts of config. I suppose the point is that their
> comment characters wouldn't make it into the commit object at all, so
> the right answer there is probably not to look for comment characters at
> all.

I see some sort of over-wrap between 'git stripspace --strip-comments' and 'git interpret-trailers'

The 'commit-msg' hook could pass the message to 'git interpret-trailers' after trimming all comment lines by using 'git stripspace --strip-comments'.

Anyway, users' hooks already rely on the behavior that 'git interpret-trailers' ignores lines starting with '#'.

So, respecting core.commentChar is the right fix, I believe.

BTW, why is commit-msg hook is passed with a message without comments unstripped? I think the straightforward behavior would be to strip comments first, then invoke the commit hook.

Thanks.

-- 
Best Regards
Masahiro Yamada