Re: 2.19.0 regression: leaving editor with empty commit message doesn't stop rebase [was: Re: [RFC PATCH v5 9/9] git-rebase: make --allow-empty-message the default]

Hi,

On Tue, Sep 11, 2018 at 7:42 PM SZEDER Gábor [off-list ref] wrote:

Lately I noticed that occasionally I ended up with an empty commit
message after an interactive rebase...those empty commit messages are the
consequence of a regression in v2.19.0, which bisects down to this
patch.

To reproduce the issue, just start an interactive rebase, choose a
commit to reword, save, delete all the commit message, save, and BAM!
there is the commit with the empty message.


On Wed, Jun 27, 2018 at 12:23:19AM -0700, Elijah Newren wrote:

quoted

rebase backends currently behave differently with empty commit messages,
largely as a side-effect of the different underlying commands on which
they are based.  am-based rebases apply commits with an empty commit

...

I agree that this is an issue that should be addressed, and also agree
that it's reasonable to accept an empty commit message, if the
original commit already had an empty commit message.  (Though perhaps
not completely silently, but with a warning?  Dunno.)

However, this should only be the case for existing commit messages
that are taken verbatim, but never in the case when the user is asked
for a commit message.

...

  # Please enter the commit message for your changes. Lines starting
  # with '#' will be ignored, and an empty message aborts the commit.

...

Let's suppose you start an interactive rebase, choose a commit to
squash, save the instruction sheet, rebase fires up your editor, and
then you notice that you mistakenly chose the wrong commit to squash.
What do you do, how do you abort?

All sound like reasonable arguments to me to differentiate between
commit messages that started empty (which I admit was what I had in
mind) vs. ones where we asked for user input and it came out empty.

Are you cooking up a patch?  I might be able to find a little time to
dig into this tomorrow if no one else is working on it already by
then.  It'll probably conflict a bunch with all the
rewrite-rebase-in-C topics in flight in pu.  :-(