Skip to content

Commit

Permalink
interpret-trailers: handle message without trailing newline
Browse files Browse the repository at this point in the history
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.

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:

    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.
  • Loading branch information
zivarah committed Sep 5, 2024
1 parent 6809f8c commit 2cc5184
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 5 deletions.
12 changes: 7 additions & 5 deletions builtin/interpret-trailers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
40 changes: 40 additions & 0 deletions t/t7513-interpret-trailers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions trailer.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ 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.
*/
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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions trailer.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ 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
*/
int message_without_trailing_newline_before_trailer_block(struct trailer_info *);

/*
* Free trailer_info struct.
*/
Expand Down

0 comments on commit 2cc5184

Please sign in to comment.