format-patch: Generate a newline between the subject header and the message body.

PATCH In review Robert Shearman 3 participants 6 messages Jun 15, 2016

Share & tools

Robert Shearman

author

2016-06-15 22:42 UTC #0 lore raw

PATCH 1/1 format-patch: Generate a newline between the subject header and the message body.

format-patch previously didn't generate a newline after a subject. This caused the diffstat to not be displayed in messages without a blank line and the first blank line to be eaten in messages with a blank line.

This patch inserts a newline in two places - once in the loop to separate the subject part of the commit message from the body part of the commit message and another after the loop to counteract the eating of whitespace at the end of the message.

---
 commit.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

commit.c +12 -2

index 522a6f3..8869b0d 100644
@@ -655,6 +655,9 @@ unsigned long pretty_print_commit(enum c
 			continue;
 		}
 
+		if (!subject)
+			body = 1;
+
 		if (is_empty_line(line, &linelen)) {
 			if (!body)
 				continue;

@@ -662,8 +665,6 @@ unsigned long pretty_print_commit(enum c
 				continue;
 			if (fmt == CMIT_FMT_SHORT)
 				break;
-		} else {
-			body = 1;
 		}
 
 		if (subject) {

@@ -694,6 +695,9 @@ unsigned long pretty_print_commit(enum c
 			memcpy(buf + offset, after_subject, slen);
 			offset += slen;
 			after_subject = NULL;
+		} else if (fmt == CMIT_FMT_EMAIL && subject) {
+			/* separate the headers from the body */
+			buf[offset++] = '\n';
 		}
 		subject = NULL;
 	}

@@ -702,6 +706,12 @@ unsigned long pretty_print_commit(enum c
 	/* Make sure there is an EOLN for the non-oneline case */
 	if (fmt != CMIT_FMT_ONELINE)
 		buf[offset++] = '\n';
+	/*
+	 * make sure there is another EOLN to separate the headers from whatever
+	 * body the caller appends if we haven't already written a body
+	 */
+	if (fmt == CMIT_FMT_EMAIL && !body)
+		buf[offset++] = '\n';
 	buf[offset] = '\0';
 	return offset;
 }

Junio C Hamano

2016-06-15 22:42 UTC #1 lore raw

Quoted 4 lines · Robert Shearman

This patch inserts a newline in two places - once in the loop to separate the subject part of the commit message from the body part of the commit message and another after the loop to counteract the eating of whitespace at the end of the message.

Thanks.

* Please sign your patch.

* This breaks a handful t4013 tests, but all in a good way (in other words, the expected output files were wrong).

I'll fix up the t/t4013/diff.* files myself.

Junio C Hamano

2016-06-15 22:42 UTC #2 lore raw

Quoted 7 lines · Junio C Hamano

Quoted 4 lines · Robert Shearman
This patch inserts a newline in two places - once in the loop to separate the subject part of the commit message from the body part of the commit message and another after the loop to counteract the eating of whitespace at the end of the message.

Quoted 8 lines

Thanks.
* Please sign your patch.
* This breaks a handful t4013 tests, but all in a good way (in other words, the expected output files were wrong).
I'll fix up the t/t4013/diff.* files myself.

Actually, I take that back. This breaks normal commit log messages by adding extra blank lines. The extra LF is needed when the commit log does not have any message (just title).

I'll be tweaking a few tests in t4013 to catch future breakage of this kind.

Robert Shearman

author

2016-06-15 22:42 UTC #3 lore raw

Quoted 21 lines · Junio C Hamano

Quoted 17 lines · Junio C Hamano
Quoted 6 lines · Robert Shearman
This patch inserts a newline in two places - once in the loop to separate the subject part of the commit message from the body part of the commit message and another after the loop to counteract the eating of whitespace at the end of the message.
Thanks.
* Please sign your patch.
* This breaks a handful t4013 tests, but all in a good way (in other words, the expected output files were wrong).

Quoted 9 lines

Quoted 3 lines
I'll fix up the t/t4013/diff.* files myself.
Actually, I take that back. This breaks normal commit log messages by adding extra blank lines. The extra LF is needed when the commit log does not have any message (just title).

Ok, I'll resend with that fixed.

Quoted 3 lines

I'll be tweaking a few tests in t4013 to catch future breakage of this kind.

Thanks.

Signature · 1 line

Rob Shearman

Jakub Narebski

2016-06-15 22:42 UTC #4 lore raw

In reply to Robert Shearman

Quoted 4 lines · Robert Shearman

format-patch previously didn't generate a newline after a subject. This caused the diffstat to not be displayed in messages without a blank line and the first blank line to be eaten in messages with a blank line.

Does this _enforce_ separating commit message into subject+empty line+description? What about commit messages without this structire (e.g. legacy commit messages from import from other SCM, e.g. GNU ChangeLog style)?

Signature · 3 lines

Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Robert Shearman

author

2016-06-15 22:42 UTC #5 lore raw

Quoted 12 lines · Jakub Narebski

Quoted 3 lines · Robert Shearman
format-patch previously didn't generate a newline after a subject. This caused the diffstat to not be displayed in messages without a blank line and the first blank line to be eaten in messages with a blank line.
Does this _enforce_ separating commit message into subject+empty line+description? What about commit messages without this structire (e.g. legacy commit messages from import from other SCM, e.g. GNU ChangeLog style)?

It only affects commits exported into email style. It has nothing to do with the structure of GIT commit messages or those of any other SCM.

Signature · 1 line

Rob Shearman