Re: [PATCH] commit: --amend -m '' silently fails to wipe message

From: Jeff King <peff@peff.net>
Date: Apr 7, 2016, 04:42 UTC
Message-ID: <20160407044219.GA29710@sigill.intra.peff.net>
In-Reply-To: <20160406171503.GA2345@dinwoodie.org>

On Wed, Apr 06, 2016 at 06:15:03PM +0100, Adam Dinwoodie wrote:

Show 6 quoted lines

> `git commit --amend -m ''` seems to be an unambiguous request to blank a
> commit message, but it actually leaves the commit message as-is.  That's
> the case regardless of whether `--allow-empty-message` is specified, and
> doesn't so much as drop a non-zero return code.
> 
> Add failing tests to show this behaviour.

Hmm. Is it just that we check "message.len", which cannot tell the difference between "-m was not given" and "-m was given the empty string"?

IOW, would this fix it?

diff --git a/builtin/commit.c b/builtin/commit.c
index 109742e..3cdc44e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -695,7 +695,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		}
 	}
 
-	if (message.len) {
+	if (have_option_m) {
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {

It passes the test suite and your two new tests, but I didn't look too
deeply to see if there are other fallouts. I don't think anybody should
be using it to counteract a previous "-m" or anything like that; we have
"--no-message" for that.

-Peff

PS Is there a previous thread? I see a couple people cc'd, including me,
   but I don't remember a previous discussion. Did I just forget it?