Re: [PATCH] notes: remove trailing whitespace from editor template

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Kristoffer

On 24/05/2025 22:35, kristofferhaugsbakk@fastmail.com wrote:
> 
> diff --git a/builtin/notes.c b/builtin/notes.c
> index a3f433ca4c0..ca4782eca19 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)
>   	if (strbuf_read(&buf, show.out, 0) < 0)
>   		die_errno(_("could not read 'show' output"));
>   	strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);
> +	/* strip trailing whitespace introduced by blank lines */
> +	strbuf_stripspace(&cbuf, NULL);

It doesn't make any difference at the moment but I'd be happier if we 
stripped the trailing space from the commit message before commenting it 
out. That way we know we are only stripping space from the indented 
lines produced by "git show". If in the future this function were to 
start appending the commented log message to a buffer passed in by the 
caller rather than a file passed by the caller we wont mess up the rest 
of the buffer content.

>   	write_or_die(fd, cbuf.buf, cbuf.len);
 > [...]> +test_expect_success 'git notes add has no trailing whitespace 
in the editor template' '
> +	test_commit --signoff 23rd &&
> +	GIT_EDITOR="cat >actual" git notes add &&
> +	test_grep ! " $" actual

Should that be " \$"? What you've got seems to work with dash but I'm 
not sure if it is POSIX compliant or not.

Best Wishes

Phillip