[PATCH v2/RFC] commit: change the meaning of an empty commit message

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Date: Aug 21, 2017, 14:05 UTC
Message-ID: <20170821140528.7212-1-kaarticsivaraam91196@gmail.com>
In-Reply-To: <1500107583.1850.4.camel@gmail.com>

An "empty commit message" according to 'commit' has long been,

    A message that contains only empty lines and/or whitespaces
    and/or 'Signed-off-by' lines

This is biased as a commit message that contains only other trailers like 'Helped-by: ', 'Tested-by: ' etc., could equally be considered empty but such messages are considered valid. Detecting *all* possible trailers and aborting when a commit message contains only those trailers is not an easy thing as the meaning of a 'trailer' is not universal.

Further, leaving the meaning unchanged has the issue that it isn't consistent with the meaning of an empty "merge" message which is,

    A message that contains only empty lines and/or whitespaces

In order to keep the implementation simple and to be consistent with the meaning of an "empty merge message"and to remain unbiased redefine the meaning of an "empty commit message" as,

    A message that contains only empty lines and/or whitespaces

Users who would like to have a different notion of an "empty commit message" can do so using the 'commit-msg' hook.

As a result of this change, the following commit message which was rejected as empty before this change is considered to be valid as a consequence of this change.

            ----   START : COMMIT MESSAGE ----

    Signed-off-by: Random J Developer <developer@example.org>

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

With the default cleanup, the above message would produce a commit with the 'Signed-off-by:' line as it's subject. Eg,

    [master 4a34e74] Signed-off-by: Random J Developer <developer@example.org>

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---

 As has been noted by Junio,

     "It would be a backward incompatible tightening of the established
     rule, but it may not be a bad change."

 The "It" above refers to this change. Expecting comments from people to ensure
 this change isn't a bad one.

 Changes in v2:

    Unlike the previous patch this one "doesn't add much". Only the meaning of
    the empty commit message has been changed.

    Unlike the previous patch, this one doesn't touch on 'merge' because after
    this patch has been applied both commit and merge seem to reject the same set
    of messages as an empty message.

    I couldn't find the meaning of an empty commit message in any part of the
    documentation. Let me know if there's some doc to update.

 builtin/commit.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 8e9380251..26636aac1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -981,7 +981,7 @@ static int rest_is_empty(struct strbuf *sb, int start)
 	int i, eol;
 	const char *nl;
 
-	/* Check if the rest is just whitespace and Signed-off-by's. */
+	/* Check if the rest is just whitespace */
 	for (i = start; i < sb->len; i++) {
 		nl = memchr(sb->buf + i, '\n', sb->len - i);
 		if (nl)
@@ -989,11 +989,6 @@ static int rest_is_empty(struct strbuf *sb, int start)
 		else
 			eol = sb->len;
 
-		if (strlen(sign_off_header) <= eol - i &&
-		    starts_with(sb->buf + i, sign_off_header)) {
-			i = eol;
-			continue;
-		}
 		while (i < eol)
 			if (!isspace(sb->buf[i++]))
 				return 0;
@@ -1003,8 +998,7 @@ static int rest_is_empty(struct strbuf *sb, int start)
 }
 
 /*
- * Find out if the message in the strbuf contains only whitespace and
- * Signed-off-by lines.
+ * Find out if the message in the strbuf contains only whitespace
  */
 static int message_is_empty(struct strbuf *sb)
 {

-- 
2.14.1.656.g66e7d6d0f