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

From: Junio C Hamano <gitster@pobox.com>
Date: May 27, 2025, 17:18 UTC
Message-ID: <xmqq5xhmvuol.fsf@gitster.g>
In-Reply-To: <66e92d69-8372-47cf-a350-95365f72ca1c@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 17 quoted lines

> 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.

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.