git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] interpret-trailers: handle message without trailing newline

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Sep 9, 2024, 09:13 UTC
Message-ID
<ca753ef4-c567-44c3-b4c3-8e483a9a3a95@gmail.com>
In-Reply-To
<20240906145743.2059405-1-brianmlyles@gmail.com>
Hi Brian
This version looks good to me
Thanks
Phillip
On 06/09/2024 15:50, Brian Lyles wrote:
Show 103 quoted lines
> 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.
> 
> Use `strbuf_complete_line` to ensure that the message ends with a
> newline character when reading the input.
> 
> Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
> ---
> 
> Differences from v2:
> - We now use `strbuf_complete_line` when reading the input file instead
>    of handling the lack of a newline when constructing the output, which
>    drastically simplifies the patch. Thanks to Phillip for this
>    suggestion.
> - Removed some unnecessary `\` in the new tests.
> 
> The range-diff from v2 is not included since the patch is so different
> that range-diff is not able to provide anything meaningful.
> 
> 
>   builtin/interpret-trailers.c  |  1 +
>   t/t7513-interpret-trailers.sh | 40 +++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
> 
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 1d969494cf..e6f22459f1 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -132,6 +132,7 @@ static void read_input_file(struct strbuf *sb, const char *file)
>   		if (strbuf_read(sb, fileno(stdin), 0) < 0)
>   			die_errno(_("could not read from stdin"));
>   	}
> +	strbuf_complete_line(sb);
>   }
> 
>   static void interpret_trailers(const struct process_trailer_options *opts,
> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
> index 3d3e13ccf8..d78cae3e04 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
> --
> 2.45.2
> 
Previous: Junio C Hamano
Message 10 of 10 in “interpret-trailers: handle message without trailing newline”
  1. interpret-trailers: handle message without trailing newlineBrian Lyles, Sep 5, 2024
  2. Brian LylesSep 5, 2024
  3. interpret-trailers: handle message without trailing newlineBrian Lyles, Sep 6, 2024
  4. Phillip WoodSep 6, 2024
  5. Junio C HamanoSep 6, 2024
  6. interpret-trailers: handle message without trailing newlineBrian Lyles, Sep 6, 2024
  7. Junio C HamanoSep 6, 2024
  8. Phillip WoodSep 9, 2024
  9. Junio C HamanoSep 9, 2024
  10. Phillip WoodSep 9, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.