[PATCH] mailinfo: don't trim whitespace in the commit message

From

Lukas Sandström <luksan@gmail.com>

Date

Feb 19, 2010, 11:54 UTC

Message-ID

<4B7E7BDA.4040701@gmail.com>

In-Reply-To

<alpine.LFD.2.00.1002180936240.4141@localhost.localdomain>

Signed-off-by: Lukas Sandström <luksan@gmail.com>
---

On 2010-02-18 19:05, Linus Torvalds wrote:

> 'git mailinfo' removes the whitespace from the beginning of the email
> body, but it does it incorrectly.
>
> In particular, some people use indented paragraphs, like this:
>
> 	  Four-score and Four score and seven years ago our fathers
>    brought forth, upon this continent, a new nation, conceived in Liberty,
>    and dedicated to the proposition that all men are created equal.
>
> 	Now we are engaged in a great civil war, testing whether that
>    nation, or any nation so conceived, and so dedicated, can long endure.
>    We are met here on a great battlefield of that war. We have come to
>    dedicate a portion of it as a final resting place for those who here
>    gave their lives that that nation might live. It is altogether fitting
>    and proper that we should do this.
>
>    ...
>
> and mailinfo will not just remove empty lines from the beginning of the
> email body, it will also remove the _first_ indentation (but not any
> others). Which makes the whole thing come out wrong.
>
> I bisected it, and this bug was introduced almost two years ago. In commit
> 3b6121f69b2 ("git-mailinfo: use strbuf's instead of fixed buffers"), to be
> exact. I'm pretty sure the bug is that handle_commit_msg() was changed to
> use 'strbuf_ltrim()' for the 'still_looking' case.
>
> Before commit 3b6121f69b2, it would create a new variable that had the
> trimmed results ("char *cp = line;"), after that commit it would just trim
> the line itself. Which is correct for the case of it being a header, but
> if it's the first non-header line, it's wrong.
>

/Lukas

 builtin-mailinfo.c |   41 ++++++++++++++++++++++-------------------
 t/t5100/msg0015    |    2 +-
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index a50ac22..954dc11 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -283,11 +283,15 @@ static inline int cmp_header(const struct strbuf *line, const char *hdr)
 			line->buf[len] == ':' && isspace(line->buf[len + 1]);
 }

-static int check_header(const struct strbuf *line,
+static int check_header(const struct strbuf *line_in,
 				struct strbuf *hdr_data[], int overwrite)
 {
 	int i, ret = 0, len;
-	struct strbuf sb = STRBUF_INIT;
+	struct strbuf sb = STRBUF_INIT, sb2 = STRBUF_INIT, *line = &sb2;
+
+	strbuf_addbuf(line, line_in);
+	strbuf_ltrim(line);
+
 	/* search for the interesting parts */
 	for (i = 0; header[i]; i++) {
 		int len = strlen(header[i]);
@@ -339,6 +343,7 @@ static int check_header(const struct strbuf *line,

 check_header_out:
 	strbuf_release(&sb);
+	strbuf_release(&sb2);
 	return ret;
 }

@@ -773,27 +778,25 @@ static int is_scissors_line(const struct strbuf *line)

 static int handle_commit_msg(struct strbuf *line)
 {
-	static int still_looking = 1;
+	static int first_msg_line_found = 0;
+	size_t cnt;

 	if (!cmitmsg)
-		return 0;
-
-	if (still_looking) {
-		strbuf_ltrim(line);
-		if (!line->len)
+		return 0; /* FIXME: shouldn't this be: return 1? */
+
+	if (!first_msg_line_found) {
+		if (use_inbody_headers)
+			if (check_header(line, s_hdr_data, 0))
+				return 0;
+		/* Check if the first line is all whitespace */
+		for (cnt = 0; isspace(line->buf[cnt]); cnt++)
+			; /* nothing */
+		if (line->len == cnt)
+			/* Ignore the first line if it's only whitespace */
 			return 0;
+		first_msg_line_found = 1;
 	}

-	if (use_inbody_headers && still_looking) {
-		still_looking = check_header(line, s_hdr_data, 0);
-		if (still_looking)
-			return 0;
-	} else
-		/* Only trim the first (blank) line of the commit message
-		 * when ignoring in-body headers.
-		 */
-		still_looking = 0;
-
 	/* normalize the log message to UTF-8. */
 	if (metainfo_charset)
 		convert_to_utf8(line, charset.buf);
@@ -804,7 +807,7 @@ static int handle_commit_msg(struct strbuf *line)
 			die_errno("Could not rewind output message file");
 		if (ftruncate(fileno(cmitmsg), 0))
 			die_errno("Could not truncate output message file at scissors");
-		still_looking = 1;
+		first_msg_line_found = 0;

 		/*
 		 * We may have already read "secondary headers"; purge
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
index 9577238..4abb3d5 100644
--- a/t/t5100/msg0015
+++ b/t/t5100/msg0015
@@ -1,2 +1,2 @@
-- a list
+  - a list
   - of stuff

-- 
1.6.6.1