When git-interpret-trailers is used to add a trailer to a message that
does not end in a trailing newline, the new trailer is added on the line
immediately following the message instead of as a trailer block
separated from the message by a blank line.
For example, if a message's text was exactly "The subject" with no
trailing newline present, `git interpret-trailers --trailer
my-trailer=true` will result in the following malformed commit message:
The subject
my-trailer: true
While it is generally expected that a commit message should end with a
newline character, git-interpret-trailers should not be returning an
invalid message in this case.
Detect when a message exists but does not end with a newline character,
and add an extra newline before appending the new trailer.
Signed-off-by: Brian Lyles <brianmlyles@xxxxxxxxx>
---
Differences from v1:
- Minor tweak to commit message wording
- Updated stale documentation from initial prototype to accurately
reflect the actual state of the code.
Range-diff vs v1:
1: 7f67f06a08 ! 1: af02465f86 interpret-trailers: handle message without trailing newline
@@ Commit message
When git-interpret-trailers is used to add a trailer to a message that
does not end in a trailing newline, the new trailer is added on the line
immediately following the message instead of as a trailer block
- separated from the message by a newline character.
+ separated from the message by a blank line.
- For example, if a message's text was "The subject" with no trailing
- newline at all, `git interpret-trailers --trailer my-trailer=true` will
- result in the following malformed commit message:
+ For example, if a message's text was exactly "The subject" with no
+ trailing newline present, `git interpret-trailers --trailer
+ my-trailer=true` will result in the following malformed commit message:
The subject
my-trailer: true
@@ trailer.c: struct trailer_info {
int blank_line_before_trailer;
+ /*
-+ * True if the last character before the location pointed to be
-+ * trailer_block_start is a newline character.
++ * True if there is a message before the trailer block and it does not
++ * end with a trailing newline character.
+ */
+ int message_without_trailing_newline_before_trailer;
+
@@ trailer.h: size_t trailer_block_end(struct trailer_info *);
int blank_line_before_trailer_block(struct trailer_info *);
+/*
-+ * Return 1 if the trailer block had a newline character
++ * Return true if there is a message before the trailer block and it does not
++ * end with a trailing newline character.
+ */
+int message_without_trailing_newline_before_trailer_block(struct trailer_info *);
+
builtin/interpret-trailers.c | 12 ++++++-----
t/t7513-interpret-trailers.sh | 40 +++++++++++++++++++++++++++++++++++
trailer.c | 18 ++++++++++++++++
trailer.h | 6 ++++++
4 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 1d969494cf..9d8f94341d 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -153,13 +153,15 @@ static void interpret_trailers(const struct process_trailer_options *opts,
info = parse_trailers(opts, sb.buf, &head);
- /* Print the lines before the trailers */
- if (!opts->only_trailers)
+ if (!opts->only_trailers) {
+ /* Print the lines before the trailers */
fwrite(sb.buf, 1, trailer_block_start(info), outfile);
- if (!opts->only_trailers && !blank_line_before_trailer_block(info))
- fprintf(outfile, "\n");
-
+ if (message_without_trailing_newline_before_trailer_block(info))
+ fprintf(outfile, "\n\n");
+ else if (!blank_line_before_trailer_block(info))
+ fprintf(outfile, "\n");
+ }
if (!opts->only_input) {
LIST_HEAD(config_head);
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 3d3e13ccf8..d5303c3f74 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -175,6 +175,46 @@ test_expect_success 'with only a title in the message' '
test_cmp expected actual
'
+test_expect_success 'with a bodiless message that lacks a trailing newline after the subject' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change" | \
+ git interpret-trailers --trailer "Reviewed-by: Peff" \
+ --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with a bodied message that lacks a trailing newline after the body' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ details about the change.
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change\n\ndetails about the change." | \
+ git interpret-trailers --trailer "Reviewed-by: Peff" \
+ --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with a message that lacks a trailing newline after the trailers' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change\n\nReviewed-by: Peff" | \
+ git interpret-trailers --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with multiline title in the message' '
cat >expected <<-\EOF &&
place of
diff --git a/trailer.c b/trailer.c
index 72e5136c73..178adae6ad 100644
--- a/trailer.c
+++ b/trailer.c
@@ -18,6 +18,12 @@ struct trailer_info {
*/
int blank_line_before_trailer;
+ /*
+ * True if there is a message before the trailer block and it does not
+ * end with a trailing newline character.
+ */
+ int message_without_trailing_newline_before_trailer;
+
/*
* Offsets to the trailer block start and end positions in the input
* string. If no trailer block is found, these are both set to the
@@ -946,6 +952,11 @@ static int ends_with_blank_line(const char *buf, size_t len)
return is_blank_line(buf + ll);
}
+static int has_message_without_trailing_newline_char(const char *buf, size_t len)
+{
+ return len > 0 && buf[len - 1] != '\n';
+}
+
static void unfold_value(struct strbuf *val)
{
struct strbuf out = STRBUF_INIT;
@@ -1017,6 +1028,8 @@ static struct trailer_info *trailer_info_get(const struct process_trailer_option
info->blank_line_before_trailer = ends_with_blank_line(str,
trailer_block_start);
+ info->message_without_trailing_newline_before_trailer
+ = has_message_without_trailing_newline_char(str, trailer_block_start);
info->trailer_block_start = trailer_block_start;
info->trailer_block_end = end_of_log_message;
info->trailers = trailer_strings;
@@ -1090,6 +1103,11 @@ int blank_line_before_trailer_block(struct trailer_info *info)
return info->blank_line_before_trailer;
}
+int message_without_trailing_newline_before_trailer_block(struct trailer_info *info)
+{
+ return info->message_without_trailing_newline_before_trailer;
+}
+
void trailer_info_release(struct trailer_info *info)
{
size_t i;
diff --git a/trailer.h b/trailer.h
index 6eb53df155..81449151c7 100644
--- a/trailer.h
+++ b/trailer.h
@@ -125,6 +125,12 @@ size_t trailer_block_end(struct trailer_info *);
*/
int blank_line_before_trailer_block(struct trailer_info *);
+/*
+ * Return true if there is a message before the trailer block and it does not
+ * end with a trailing newline character.
+ */
+int message_without_trailing_newline_before_trailer_block(struct trailer_info *);
+
/*
* Free trailer_info struct.
*/
--
2.45.2
