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

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 17, 2019, 20:07 UTC
Message-ID: <CAP8UFD3EaR=PRsN9sEEAOOdqQxPeZ1wiiWrVeBKQv8AW1zY1ig@mail.gmail.com>
In-Reply-To: <xmqqtvcoruda.fsf@gitster-ct.c.googlers.com>

On Mon, Jun 17, 2019 at 7:31 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 31 quoted lines

>
> 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.

Thanks, Christian.