Hi there Phillip
On Mon, May 26, 2025, at 16:01, Phillip Wood 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.
Do you mean doing the operation on the output buffer instead?:
if (strbuf_read(&buf, show.out, 0) < 0)
die_errno(_("could not read 'show' output"));
/* strip trailing whitespace introduced by blank lines */
strbuf_stripspace(&buf, NULL);
strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);
write_or_die(fd, cbuf.buf, cbuf.len);
I think that’s cleaner. But I don’t see how it makes the code more
future-proof.
>
>> 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.
`$` is the anchor metacharacter in this context (end of string)
according to Posix.
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03
$
The <dollar-sign> shall be special when used as an anchor.
