git cherry-pick -x: missing blank line before the "cherry picked from commit" line

git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Vincent Lefevre]

With git 2.38.1 under Debian, when I use "git cherry-pick -x"
on a commit such that the last line of the log message starts
with "note:" (case insensitively), the blank line before the
"cherry picked from commit" line is missing.

For instance:

    New commit
    
    foo
    
    nOtE:
    (cherry picked from commit 3ba643e2eec4bdc1cd46b478ab36ee0707d241c2)

and

    New commit
    
    Note: foo.
    (cherry picked from commit d0e85cdd32e30f78eeb968f275fc3a98899d791e)

but the following ones are OK:

    New commit
    
    note:
    foo
    
    (cherry picked from commit a0ffae22fd3c94210170a3addcf802804f6ee5f7)

----

    New commit
    
    A Note:
    
    (cherry picked from commit 0a89e0e7f8be3063803b0ad4381cd848ec52dd39)

----

    New commit
    
    Note
    
    (cherry picked from commit f217bd5069c9d66a8ca54c869919ff484a18d20c)

----

I also had reported the bug in the Debian BTS:

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025155

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Phillip Wood]

Hi Vincent

On 02/12/2022 14:11, Vincent Lefevre wrote:
> With git 2.38.1 under Debian, when I use "git cherry-pick -x"
> on a commit such that the last line of the log message starts
> with "note:" (case insensitively), the blank line before the
> "cherry picked from commit" line is missing.
> 
> For instance:
> 
>      New commit
>      
>      foo
>      
>      nOtE:
>      (cherry picked from commit 3ba643e2eec4bdc1cd46b478ab36ee0707d241c2)

"git cherry-pick -x" takes care not to add a blank line if there are 
trailers such as Signed-off-by: at the end of the message (if it did 
then the trailers would no longer be recognized as trailers as they 
would not be the last paragraph). In this case the "nOtE:" line looks 
like a trailer and so no blank line is added.

I can see it is annoying in this case but I'm afraid I don't have any 
ideas of how we could avoid it without breaking the case where the 
message does contain trailers.

Best Wishes

Phillip

> and
> 
>      New commit
>      
>      Note: foo.
>      (cherry picked from commit d0e85cdd32e30f78eeb968f275fc3a98899d791e)
> 
> but the following ones are OK:
> 
>      New commit
>      
>      note:
>      foo
>      
>      (cherry picked from commit a0ffae22fd3c94210170a3addcf802804f6ee5f7)
> 
> ----
> 
>      New commit
>      
>      A Note:
>      
>      (cherry picked from commit 0a89e0e7f8be3063803b0ad4381cd848ec52dd39)
> 
> ----
> 
>      New commit
>      
>      Note
>      
>      (cherry picked from commit f217bd5069c9d66a8ca54c869919ff484a18d20c)
> 
> ----
> 
> I also had reported the bug in the Debian BTS:
> 
>    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025155
> 

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Vincent Lefevre]

Hi Phillip,

On 2022-12-02 16:41:23 +0000, Phillip Wood wrote:
> On 02/12/2022 14:11, Vincent Lefevre wrote:
> > With git 2.38.1 under Debian, when I use "git cherry-pick -x"
> > on a commit such that the last line of the log message starts
> > with "note:" (case insensitively), the blank line before the
> > "cherry picked from commit" line is missing.
> > 
> > For instance:
> > 
> >      New commit
> >      foo
> >      nOtE:
> >      (cherry picked from commit 3ba643e2eec4bdc1cd46b478ab36ee0707d241c2)
> 
> "git cherry-pick -x" takes care not to add a blank line if there are
> trailers such as Signed-off-by: at the end of the message (if it did then
> the trailers would no longer be recognized as trailers as they would not be
> the last paragraph). In this case the "nOtE:" line looks like a trailer and
> so no blank line is added.
> 
> I can see it is annoying in this case but I'm afraid I don't have any ideas
> of how we could avoid it without breaking the case where the message does
> contain trailers.

First, one issue is that this is not documented (I was not aware of
the notion of trailers, well, at least concerning a special handling
by Git).

Then perhaps there should be some configuration to define which
tokens are allowed (or forbidden) for trailers. For instance,
I would say that "Note" is too common in log messages to be
regarded as a trailer token.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Phillip Wood]

Hi Vincent

On 02/12/2022 17:01, Vincent Lefevre wrote:
> Hi Phillip,
> 
> On 2022-12-02 16:41:23 +0000, Phillip Wood wrote:
>> On 02/12/2022 14:11, Vincent Lefevre wrote:
>>> With git 2.38.1 under Debian, when I use "git cherry-pick -x"
>>> on a commit such that the last line of the log message starts
>>> with "note:" (case insensitively), the blank line before the
>>> "cherry picked from commit" line is missing.
>>>
>>> For instance:
>>>
>>>       New commit
>>>       foo
>>>       nOtE:
>>>       (cherry picked from commit 3ba643e2eec4bdc1cd46b478ab36ee0707d241c2)
>>
>> "git cherry-pick -x" takes care not to add a blank line if there are
>> trailers such as Signed-off-by: at the end of the message (if it did then
>> the trailers would no longer be recognized as trailers as they would not be
>> the last paragraph). In this case the "nOtE:" line looks like a trailer and
>> so no blank line is added.
>>
>> I can see it is annoying in this case but I'm afraid I don't have any ideas
>> of how we could avoid it without breaking the case where the message does
>> contain trailers.
> 
> First, one issue is that this is not documented (I was not aware of
> the notion of trailers, well, at least concerning a special handling
> by Git).

They are mentioned in the documentation for 'git commit' see the 
--signoff and --trailer options. The main reference is 'git 
interpret-trailers'

> Then perhaps there should be some configuration to define which
> tokens are allowed (or forbidden) for trailers. For instance,
> I would say that "Note" is too common in log messages to be
> regarded as a trailer token.

There are some safeguards when looking for trailers (see the 
interpret-trailers man page) I think you have been unlucky here, I don't 
remember this being reported before. I've cc'd Christian who knows more 
about trailers than I do to see if he has anything to add.

Best Wishes

Phillip

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Vincent Lefevre]

On 2022-12-04 15:13:52 +0000, Phillip Wood wrote:
> On 02/12/2022 17:01, Vincent Lefevre wrote:
> > First, one issue is that this is not documented (I was not aware of
> > the notion of trailers, well, at least concerning a special handling
> > by Git).
> 
> They are mentioned in the documentation for 'git commit' see the --signoff
> and --trailer options. The main reference is 'git interpret-trailers'

This issue occurs with "git cherry-pick -x", so that one could expect
documentation there. Since I don't use trailers (this was a false
positive), I wouldn't have the idea to look at the --trailer
documentation.

> > Then perhaps there should be some configuration to define which
> > tokens are allowed (or forbidden) for trailers. For instance,
> > I would say that "Note" is too common in log messages to be
> > regarded as a trailer token.
> 
> There are some safeguards when looking for trailers (see the
> interpret-trailers man page) I think you have been unlucky here, I don't
> remember this being reported before. I've cc'd Christian who knows more
> about trailers than I do to see if he has anything to add.

I think that what could be interesting there is a
trailer.<token>.separators option (assuming that one can
use an empty list of separators to disable a token), but
that doesn't exist.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Christian Couder]

On Sun, Dec 4, 2022 at 4:49 PM Vincent Lefevre <vincent@vinc17.net> wrote:
>
> On 2022-12-04 15:13:52 +0000, Phillip Wood wrote:
> > On 02/12/2022 17:01, Vincent Lefevre wrote:
> > > First, one issue is that this is not documented (I was not aware of
> > > the notion of trailers, well, at least concerning a special handling
> > > by Git).
> >
> > They are mentioned in the documentation for 'git commit' see the --signoff
> > and --trailer options. The main reference is 'git interpret-trailers'
>
> This issue occurs with "git cherry-pick -x", so that one could expect
> documentation there. Since I don't use trailers (this was a false
> positive), I wouldn't have the idea to look at the --trailer
> documentation.

Yeah, the documentation for the -x option on the git cherry-pick page
should probably say that the added line will be put at the end of the
trailer part of the commit message and perhaps link to the git commit
or git interpret-trailers page. Patch welcome!

> > > Then perhaps there should be some configuration to define which
> > > tokens are allowed (or forbidden) for trailers. For instance,
> > > I would say that "Note" is too common in log messages to be
> > > regarded as a trailer token.
> >
> > There are some safeguards when looking for trailers (see the
> > interpret-trailers man page) I think you have been unlucky here, I don't
> > remember this being reported before. I've cc'd Christian who knows more
> > about trailers than I do to see if he has anything to add.
>
> I think that what could be interesting there is a
> trailer.<token>.separators option (assuming that one can
> use an empty list of separators to disable a token), but
> that doesn't exist.

There is the trailer.separators config option. I am not sure if it
disables trailers completely though if it is empty. If it doesn't,
that could be an interesting feature to implement for people who don't
use trailers at all (except for lines added by cherry-pick).

A trailer.<token>.separators option that disables some <token> when
empty could be another interesting idea, but it might have drawbacks.
For example you might disable the "note" token, but then find that
people sometimes use "notes:" or "NB:" or "PS:" or other things like
that at the end of their commit messages.

Re: git cherry-pick -x: missing blank line before the "cherry picked from commit" line

[Vincent Lefevre]

On 2022-12-05 13:56:02 +0100, Christian Couder wrote:
> On Sun, Dec 4, 2022 at 4:49 PM Vincent Lefevre <vincent@vinc17.net> wrote:
> > I think that what could be interesting there is a
> > trailer.<token>.separators option (assuming that one can
> > use an empty list of separators to disable a token), but
> > that doesn't exist.
> 
> There is the trailer.separators config option. I am not sure if it
> disables trailers completely though if it is empty. If it doesn't,
> that could be an interesting feature to implement for people who don't
> use trailers at all (except for lines added by cherry-pick).

I have not tried yet, but the drawback is that it could be used
only if trailers are not used at all.

> A trailer.<token>.separators option that disables some <token> when
> empty could be another interesting idea, but it might have drawbacks.
> For example you might disable the "note" token, but then find that
> people sometimes use "notes:" or "NB:" or "PS:" or other things like
> that at the end of their commit messages.

Yes, or "References:" or "https:". Like that, which has just happened
to me: :-(

[...]
    References:
      https://sympa.inria.fr/sympa/arc/mpfr/2022-10/msg00001.html
      https://sympa.inria.fr/sympa/arc/mpfr/2022-10/msg00027.html
      https://gmplib.org/list-archives/gmp-bugs/2022-October/005200.html
    (cherry picked from commit 277faface2023449c91c1751630e760d136ae619)

Even when one knows the issue, it is too easy to forget it.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)