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

[Masahiro Yamada <yamada.masahiro@socionext.com>]

On Tue, Jun 18, 2019 at 5:08 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Mon, Jun 17, 2019 at 7:31 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Christian Couder <christian.couder@gmail.com> writes:
> >
> > > On Mon, Jun 17, 2019 at 6:33 AM Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > >>
> > >> On Sat, Jun 15, 2019 at 5:41 PM Christian Couder
> > >> <christian.couder@gmail.com> wrote:
> > >> >
> > >> > > 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.
> > >> >
> > >> > Would you suggest an option, maybe called `--ignore-comments` to ignore them?
> > >>
> > >> Since 'git interpret-trailers' already ignores lines starting with '#',
> > >> is this option true by default?
> > >
> > > Sorry, I should have suggested something called --unstrip-comments or
> > > --ignore-comment-char that would make 'git interpret-trailers' stop
> > > stripping lines that start with the comment character.
> >
> > So, to summarize:
> >
> >  - As the traditional behaviour is to strip comment, using the
> >    hardcoded definition of the comment char, i.e. '#', we do not
> >    switch the default.  Instead, a new command line option makes
> >    it pretend there is no comment char and nothing get stripped.
>
> Yeah, that's the idea of --unstrip-comments (or
> --ignore-comment-char). I am ok with preparing and sending a patch to
> add that, though it is not urgent and it would be nice if we could
> agree with the name first.
>
> >  - But the core.commentchar that does not override hardcoded
> >    definition is a bug, so we'd fix that along the lines of what
> >    Peff's patch outlined.
>
> Yeah, not sure if Peff wants to resend his patch with a proper commit
> message. I would be ok with doing it if he prefers.


Sounds good to me.

These are separate works.

Since the second one is a bug-fix, it can go in first.
(Peff's patch works for me)


The first one is a new feature, so we can take our time
to decide a preferred option name.




IMHO, --unstrip-* is a little bit confusing.

interpret-trailers does not strip anything from the output.

It is just like interpret-trailers does not take comment lines
into account when it determines the boundary between the commit
message body and trailers.


Just a idea:

--[no-]ignore-comments

   By default, comments (i.e. lines starting with '#' or a character
specified core.commentChar)
   are not taken into consideration when interpret-trailers determines which
   parts are trailers.
   Pass --no-ignore-comments if you want to treat all lines as the message body.
   --ignore-comments is the default.



Please feel free to put your ideas on the table!


-- 
Best Regards
Masahiro Yamada