patch, 5 partstrailer: ignore comment lines inside the trailers

diff --git a/trailer.c b/trailer.c
index 8514566..761b763 100644
--- a/trailer.c
+++ b/trailer.c
@@ -804,8 +804,10 @@ static int process_input_file(struct strbuf **lines,
 
 	/* Parse trailer lines */
 	for (i = trailer_start; i < patch_start; i++) {
-		struct trailer_item *new = create_trailer_item(lines[i]->buf);
-		add_trailer_item(in_tok_first, in_tok_last, new);
+		if (lines[i]->buf[0] != comment_line_char) {
+			struct trailer_item *new = create_trailer_item(lines[i]->buf);
+			add_trailer_item(in_tok_first, in_tok_last, new);
+		}
 	}
 
 	return patch_start;

diff --git a/trailer.c b/trailer.c
index 761b763..f4d51ba 100644
--- a/trailer.c
+++ b/trailer.c
@@ -583,8 +583,12 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra
 	strbuf_addch(&seps, '=');
 	len = strcspn(trailer, seps.buf);
 	strbuf_release(&seps);
-	if (len == 0)
-		return error(_("empty trailer token in trailer '%s'"), trailer);
+	if (len == 0) {
+		struct strbuf sb = STRBUF_INIT;
+		strbuf_addstr(&sb, trailer);
+		strbuf_rtrim(&sb);
+		return error(_("empty trailer token in trailer '%s'"), sb.buf);
+	}
 	if (len < strlen(trailer)) {
 		strbuf_add(tok, trailer, len);
 		strbuf_trim(tok);

builtin/commit.c, commit.c, commit.h

diff --git a/builtin/commit.c b/builtin/commit.c
index e3c60dd..cda74e9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -677,52 +677,6 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	comment_line_char = *p;
 }
 
-/*
- * Inspect sb and determine the true "end" of the log message, in
- * order to find where to put a new Signed-off-by: line.  Ignored are
- * trailing comment lines and blank lines, and also the traditional
- * "Conflicts:" block that is not commented out, so that we can use
- * "git commit -s --amend" on an existing commit that forgot to remove
- * it.
- *
- * Returns the number of bytes from the tail to ignore, to be fed as
- * the second parameter to append_signoff().
- */
-static int ignore_non_trailer(struct strbuf *sb)
-{
-	int boc = 0;
-	int bol = 0;
-	int in_old_conflicts_block = 0;
-
-	while (bol < sb->len) {
-		char *next_line;
-
-		if (!(next_line = memchr(sb->buf + bol, '\n', sb->len - bol)))
-			next_line = sb->buf + sb->len;
-		else
-			next_line++;
-
-		if (sb->buf[bol] == comment_line_char || sb->buf[bol] == '\n') {
-			/* is this the first of the run of comments? */
-			if (!boc)
-				boc = bol;
-			/* otherwise, it is just continuing */
-		} else if (starts_with(sb->buf + bol, "Conflicts:\n")) {
-			in_old_conflicts_block = 1;
-			if (!boc)
-				boc = bol;
-		} else if (in_old_conflicts_block && sb->buf[bol] == '\t') {
-			; /* a pathname in the conflicts block */
-		} else if (boc) {
-			/* the previous was not trailing comment */
-			boc = 0;
-			in_old_conflicts_block = 0;
-		}
-		bol = next_line - sb->buf;
-	}
-	return boc ? sb->len - boc : 0;
-}
-
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct commit *current_head,
 			     struct wt_status *s,
diff --git a/commit.c b/commit.c
index 19cf8f9..a54cb9a 100644
--- a/commit.c
+++ b/commit.c
@@ -1640,3 +1640,49 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
 	}
 	return NULL;
 }
+
+/*
+ * Inspect sb and determine the true "end" of the log message, in
+ * order to find where to put a new Signed-off-by: line.  Ignored are
+ * trailing comment lines and blank lines, and also the traditional
+ * "Conflicts:" block that is not commented out, so that we can use
+ * "git commit -s --amend" on an existing commit that forgot to remove
+ * it.
+ *
+ * Returns the number of bytes from the tail to ignore, to be fed as
+ * the second parameter to append_signoff().
+ */
+int ignore_non_trailer(struct strbuf *sb)
+{
+	int boc = 0;
+	int bol = 0;
+	int in_old_conflicts_block = 0;
+
+	while (bol < sb->len) {
+		char *next_line;
+
+		if (!(next_line = memchr(sb->buf + bol, '\n', sb->len - bol)))
+			next_line = sb->buf + sb->len;
+		else
+			next_line++;
+
+		if (sb->buf[bol] == comment_line_char || sb->buf[bol] == '\n') {
+			/* is this the first of the run of comments? */
+			if (!boc)
+				boc = bol;
+			/* otherwise, it is just continuing */
+		} else if (starts_with(sb->buf + bol, "Conflicts:\n")) {
+			in_old_conflicts_block = 1;
+			if (!boc)
+				boc = bol;
+		} else if (in_old_conflicts_block && sb->buf[bol] == '\t') {
+			; /* a pathname in the conflicts block */
+		} else if (boc) {
+			/* the previous was not trailing comment */
+			boc = 0;
+			in_old_conflicts_block = 0;
+		}
+		bol = next_line - sb->buf;
+	}
+	return boc ? sb->len - boc : 0;
+}
diff --git a/commit.h b/commit.h
index bc68ccb..cd35ac1 100644
--- a/commit.h
+++ b/commit.h
@@ -337,6 +337,9 @@ extern void free_commit_extra_headers(struct commit_extra_header *extra);
 extern const char *find_commit_header(const char *msg, const char *key,
 				      size_t *out_len);
 
+/* Find the end of the log message, the right place for a new trailer. */
+extern int ignore_non_trailer(struct strbuf *sb);
+
 typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
 				 void *cb_data);
 

t/t7513-interpret-trailers.sh, trailer.c

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 1efb880..fed053a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -232,6 +232,8 @@ test_expect_success 'with message that has comments' '
 
 		Reviewed-by: Johan
 		Cc: Peff
+		# last comment
+
 	EOF
 	cat basic_patch >>expected &&
 	git interpret-trailers --trim-empty --trailer "Cc: Peff" message_with_comments >actual &&
diff --git a/trailer.c b/trailer.c
index f4d51ba..f6aa181 100644
--- a/trailer.c
+++ b/trailer.c
@@ -2,6 +2,7 @@
 #include "string-list.h"
 #include "run-command.h"
 #include "string-list.h"
+#include "commit.h"
 #include "trailer.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
@@ -791,14 +792,24 @@ static int process_input_file(struct strbuf **lines,
 			      struct trailer_item **in_tok_last)
 {
 	int count = 0;
-	int patch_start, trailer_start, i;
+	int trailer_start, trailer_end, patch_start, ignore_bytes, i;
+	struct strbuf sb;
 
 	/* Get the line count */
 	while (lines[count])
 		count++;
 
 	patch_start = find_patch_start(lines, count);
-	trailer_start = find_trailer_start(lines, patch_start);
+
+	/* Get the index of the end of the trailers */
+	for (i = 0; i < patch_start; i++)
+		strbuf_addbuf(&sb, lines[i]);
+	ignore_bytes = ignore_non_trailer(&sb);
+	for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--)
+		ignore_bytes -= lines[i]->len;
+	trailer_end = i + 1;
+
+	trailer_start = find_trailer_start(lines, trailer_end);
 
 	/* Print lines before the trailers as is */
 	print_lines(lines, 0, trailer_start);
@@ -807,14 +818,14 @@ static int process_input_file(struct strbuf **lines,
 		printf("\n");
 
 	/* Parse trailer lines */
-	for (i = trailer_start; i < patch_start; i++) {
+	for (i = trailer_start; i < trailer_end; i++) {
 		if (lines[i]->buf[0] != comment_line_char) {
 			struct trailer_item *new = create_trailer_item(lines[i]->buf);
 			add_trailer_item(in_tok_first, in_tok_last, new);
 		}
 	}
 
-	return patch_start;
+	return trailer_end;
 }
 
 static void free_all(struct trailer_item **first)
@@ -831,7 +842,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai
 	struct trailer_item *in_tok_last = NULL;
 	struct trailer_item *arg_tok_first;
 	struct strbuf **lines;
-	int patch_start;
+	int trailer_end;
 
 	/* Default config must be setup first */
 	git_config(git_trailer_default_config, NULL);
@@ -840,7 +851,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai
 	lines = read_input_file(file);
 
 	/* Print the lines before the trailers */
-	patch_start = process_input_file(lines, &in_tok_first, &in_tok_last);
+	trailer_end = process_input_file(lines, &in_tok_first, &in_tok_last);
 
 	arg_tok_first = process_command_line_args(trailers);
 
@@ -851,7 +862,7 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai
 	free_all(&in_tok_first);
 
 	/* Print the lines after the trailers as is */
-	print_lines(lines, patch_start, INT_MAX);
+	print_lines(lines, trailer_end, INT_MAX);
 
 	strbuf_list_free(lines);
 }

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index fed053a..bd0ab46 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -213,7 +213,7 @@ test_expect_success 'with 2 files arguments' '
 '
 
 test_expect_success 'with message that has comments' '
-	cat basic_message >>message_with_comments &&
+	cat basic_message >message_with_comments &&
 	sed -e "s/ Z\$/ /" >>message_with_comments <<-\EOF &&
 		# comment
 
@@ -240,6 +240,36 @@ test_expect_success 'with message that has comments' '
 	test_cmp expected actual
 '
 
+test_expect_success 'with message that has an old style conflict block' '
+	cat basic_message >message_with_comments &&
+	sed -e "s/ Z\$/ /" >>message_with_comments <<-\EOF &&
+		# comment
+
+		# other comment
+		Cc: Z
+		# yet another comment
+		Reviewed-by: Johan
+		Reviewed-by: Z
+		# last comment
+
+		Conflicts:
+
+	EOF
+	cat basic_message >expected &&
+	cat >>expected <<-\EOF &&
+		# comment
+
+		Reviewed-by: Johan
+		Cc: Peff
+		# last comment
+
+		Conflicts:
+
+	EOF
+	git interpret-trailers --trim-empty --trailer "Cc: Peff" message_with_comments >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'with commit complex message and trailer args' '
 	cat complex_message_body >expected &&
 	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&