Re: [PATCH] notes: remove trailing whitespace from editor template
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Jun 3, 2025, 20:37 UTC
- Message-ID
- <CALnO6CDgcQCuhxcJLH-XwxB85mxqokxsf04CU4yseTy-=XUWLQ@mail.gmail.com>
- In-Reply-To
- <xmqq5xhmvuol.fsf@gitster.g>
On Tue, May 27, 2025 at 1:31 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 33 quoted lines
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > 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?My editor doesn't complain, but it does highlight trailing whitespace at my behest, and it tends to be an eyesore (on purpose: that way I clean it up). Perhaps Kistoffer is coming from a similar place?
Show 9 quoted lines
> > 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.
I think this suggestion would also help folks who "git commit -v," which IIRC is also indented in the template.
Show 16 quoted lines
> > > 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. >
-- D. Ben Knoble