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

From: Junio C Hamano <[email-masked]>
Date: Tue, 27 May 2025 10:18:34 -0700

Phillip Wood <[email-masked]> writes:

> Hi Kristoffer
>
> On 24/05/2025 22:35, [email-masked] 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.

Yes.  I had the same thought.  If Kristof does not like the fact
that one automated source of information consistently indents its
output lines, even an empty one, and if users may have legitimate
reason to place in the final output a trailing whitespace in the
comment, it is better for the patch not to close the door to the
others.

In this case I am not all that sympathetic to the idea of the patch.
The consistently indented lines makes it more clear from which line
to which line came from a commit log message; running stripspace
would break them into paragraph pieces.  These editors that complain
probaly can be fixed?

Alternatively, if it bothers users of certain editing environments
too much, perhaps the indent code in the output phase of "git show"
should lose the indents for empty lines uniformly, shoudln't it?  It
probably should be a fairly isolated change, like the way how the
expand_tabs_in_log bit is handled in pretty.c; give another bit and
teach pp_handle_indent to return when that bit is set and the
payload it was asked to show with indentation is empty, or something
like that.

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

"2.6 Word Expansions" ends with this sentence:

    If a '$' that is neither within single-quotes nor escaped by a
    <backslash> is immediately followed by a <space>, <tab>, or a
    <newline>, or is not followed by any character, the '$' shall be
    treated as a literal character.

Taken together with "2.2.3 Double-Quotes", I'd read it as blessing a
lone '$' at the end of double-quoted string as a literal dollar sign.

Thanks.