Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

From: Brandon Casey <drafnel@gmail.com>
Date: Feb 21, 2013, 20:26 UTC
Message-ID: <CA+sFfMcNWvPKuQpNWnacegbfgN0ZP=zfuDPDRkXs1G2FMrb+iA@mail.gmail.com>
In-Reply-To: <7vip5lv6tv.fsf@alter.siamese.dyndns.org>

On Thu, Feb 21, 2013 at 10:51 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 27 quoted lines

> Brandon Casey <drafnel@gmail.com> writes:
>
>> Teach append_signoff to detect whether a blank line exists at the position
>> that the signed-off-by line will be added, and refrain from adding an
>> additional one if one already exists.  Or, add an additional line if one
>> is needed to make sure the new footer is separated from the message body
>> by a blank line.
>>
>> Signed-off-by: Brandon Casey <bcasey@nvidia.com>
>> ---
>>
>>
>> A slight tweak.  And I promise, no more are coming.
>
> When I do
>
>         $ git commit -s
>
> it should start my editor with this in the buffer:
>
>         ----------------------------------------------------------------
>
>         Signed-off-by: Junio C Hamano <gitster@pobox.com>
>         ----------------------------------------------------------------
>
> and the cursor blinking at the beginning of the file.  Annoyingly
> this step breaks it by removing the leading blank line.

Yes. The fix described by John Keeping restores the above behavior for 'commit -s'. Or the fix I described which inserts two preceding newlines so it looks like this:

   ----------------------------------------------------------------

   Signed-off-by: Junio C Hamano <gitster@pobox.com>
   ----------------------------------------------------------------

So then the cursor would be placed on the first line and a space would separate it from the sob which is arguably a better indication to the user that a blank line should separate the commit message body from the sob.

But, this does not fix the same problem for 'cherry-pick --edit -s' when used to cherry-pick a commit without a sob. The cherry-pick part of it would add the extra preceding newlines, but then cherry-pick passes the buffer to 'git commit' via .git/MERGE_MSG which then "cleans" the buffer, removing the empty lines, in prepare_to_commit() before allowing the editor to operate on it.

Using 'cherry-pick --edit -s' to cherry-pick a commit with an empty commit message is going to be a pretty rare corner case. It would be nice to have the same behavior for it that we decide to have for 'commit -s', but it's probably not worth going through contortions to make it happen.

-Brandon