Re: [PATCH v2] rebase: add --allow-empty-message option

Hi Genki,

On Sun, 4 Feb 2018, Genki Sky wrote:

This option allows commits with empty commit messages to be rebased,
matching the same option in git-commit and git-cherry-pick. While empty
log messages are frowned upon, sometimes one finds them in older
repositories (e.g. translated from another VCS [0]), or have other
reasons for desiring them. The option is available in git-commit and
git-cherry-pick, so it is natural to make other git tools play nicely
with them. Adding this as an option allows the default to be "give the
user a chance to fix", while not interrupting the user's workflow
otherwise [1].

  [0]: https://stackoverflow.com/q/8542304
  [1]: https://public-inbox.org/git/7vd33afqjh.fsf@alter.siamese.dyndns.org/

To implement this, add a new --allow-empty-message flag. Then propagate
it to all calls of 'git commit', 'git cherry-pick', and 'git rebase--helper'
within the rebase scripts.

Signed-off-by: Genki Sky <redacted>
---

Notes:

  Thanks for the initial feedback, here are the changes from [v1]:
  - Made my commit message include the main motivations inline.
  - Moved new tests to t/t3405-rebase-malformed.sh, which has the
    relevant test description: "rebase should handle arbitrary git
    message".
  - Accordingly re-used existing test setup.
  - Minimized tests to just one for git-rebase--interactive.sh and one
    for git-rebase--merge.sh. First, one test per file keeps things
    focused while getting most benefit (other code within same file is
    likely to be noticed by modifiers). And, while git-rebase--am.sh
    does have one cherry-pick, it is only for a special case with
    --keep-empty. So git-rebase--am.sh otherwise doesn't have this
    empty-message issue.

  In general, there was a concern of over-testing, and over-checking
  implementation details. So, this time, I erred on the conservative
  side.

  [v1]: https://public-inbox.org/git/9d0414b869f21f38b81f92ee0619fd76410cbcfc.1517552392.git.sky@genki.is/t/

Very nice. I looked over the patch (sorry, I have too little time to test
this thoroughly, but then, it is the custom on this here mailing list to
just review the patch as per the mail) and it looks very good to me.

Junio, if you like, please add a "Reviewed-by:" line for me.

Thanks!
Johannes