[PATCH] notes: remove trailing whitespace from editor template

[PATCH] notes: remove trailing whitespace from editor template

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

The editor template for editing a note consists of the commented block:

    git show --stat --no-notes <object>

The indented commit message will introduce trailing whitespace for
paragraph breaks (blank lines).  Some editors will highlight those lines
as an error immediately when you open the editor.

Let’s strip all unnecessary whitespace from the template to avoid that
very small problem.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 builtin/notes.c  | 2 ++
 t/t3301-notes.sh | 6 ++++++
 2 files changed, 8 insertions(+)

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);
 	write_or_die(fd, cbuf.buf, cbuf.len);
 
 	strbuf_release(&cbuf);
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d6c50460d08..70a21be54fc 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -1629,4 +1629,10 @@ test_expect_success 'git notes append aborts when editor fails with -e' '
 	test_must_fail git notes show
 '
 
+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
+'
+
 test_done

base-commit: 8613c2bb6cd16ef530dc5dd74d3b818a1ccbf1c0
-- 
2.49.0.780.g892193c3f50

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

[Kristoffer Haugsbakk]

On Sat, May 24, 2025, at 23:35, kristofferhaugsbakk@fastmail.com wrote:
> +	test_commit --signoff 23rd &&
> +	GIT_EDITOR="cat >actual" git notes add &&
> +	test_grep ! " $" actual
> +'
> +
>  test_done

Or alternatively I could assert on the whole template:

```
test_expect_success 'git notes add editor template' '
	test_commit --signoff 23rd &&
	cat <<-EOF >expect &&

	#
	# Write/edit the notes for the following object:
	#
	$(git show --stat --no-notes 23rd |
		    git stripspace |
		    git stripspace --comment-lines)
	EOF
	GIT_EDITOR="cat >actual" git notes add &&
	test_cmp expect actual
'
```

-- 
Kristoffer

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

[Phillip Wood]

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

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

[Kristoffer Haugsbakk]

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.

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

[Phillip Wood]

Hi Kristoffer

On 26/05/2025 20:44, Kristoffer Haugsbakk wrote:
> 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.

Because it is now stripping buf and not cbuf. If in the future we decide 
to build the message in a buffer rather than writing it piecemeal to 
disk we would change signature of this function to take an strbuf 
instead of a file descriptor and use the buffer provided by the caller 
instead of cbuf. If we were to strip cbuf then a naive conversion would 
end up stripping the buffer passed by caller, not just the output from 
"git show". Various git notes subcommands have a --stripspace option and 
calling strbuf_stripspace() on the caller provided buffer would break that.

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

Right but what does the shell do to that '$'? It is not escaped and 
inside a double quoted string.

Best Wishes

Phillip

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

[Kristoffer Haugsbakk]

On Tue, May 27, 2025, at 10:24, Phillip Wood wrote:
>> 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.
>
> Because it is now stripping buf and not cbuf. If in the future we decide
> to build the message in a buffer rather than writing it piecemeal to
> disk we would change signature of this function to take an strbuf
> instead of a file descriptor and use the buffer provided by the caller
> instead of cbuf. If we were to strip cbuf then a naive conversion would
> end up stripping the buffer passed by caller, not just the output from
> "git show". Various git notes subcommands have a --stripspace option and
> calling strbuf_stripspace() on the caller provided buffer would break that.

So stripping `buf` is what I should do?

>>> 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.
>
> Right but what does the shell do to that '$'? It is not escaped and
> inside a double quoted string.

Oh right, it’s about the shell.  I’ll fix it.

Thanks for spotting.

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

[Junio C Hamano]

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?

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.

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

[Eric Sunshine]

On Tue, May 27, 2025 at 1:19 PM Junio C Hamano <gitster@pobox.com> wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> > 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 for finding and posting this. I had read the POSIX shell
language specification pretty closely when writing the lexer and
parser for chainlint.pl and remembered the above statement, and had
wanted to cite it after reading Phillip's initial reply but didn't
manage to find it again in the limited time I had available for
searching through the documentation.

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

[Kristoffer Haugsbakk]

On Tue, May 27, 2025, at 19:18, Junio C Hamano wrote:
> 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.

That sounds scary. I’ll make do without.

-- 
Kristoffer Haugsbakk

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

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

An unconditional change based on the above idea may look like the
attached patch.

No tests, no docs, no configuration, and I will not be working on
this in immediate future.  On top of what we can see here, probably
the following would need to happen before we can call it completed.

 - "git notes" should take a command line option to control which
   one of "--indent-empty-lines" or "--no-indent-empty-lines" is
   passed.  Optionally, we can introduce a new configuration
   variable notes.something to control the same.

 - "git log" and "git show", but not "git rev-list" plumbing, may
   want to start paying attention to a new configuration variable
   log.indentEmptyLines (which defaults to "yes" if not specified).

   I am not sure if we want to do this item; as there is no explicit
   end-user request to add such a feature, I am inclined to say no,
   at least not as a part of the "while editing notes with 'git
   notes add -e' the shortlog ends up with lines that are commented
   out with trailing whitespaces, which upsets some editors" topic.
   But it should be trivial to do.

 - documentation.

 - tests.

I've only compiled and then manually tested

    $ GIT_EXEC_PATH=$(pwd) ./git notes add -e

to see how the commented out "git show -s" output embedded in the
editor buffer looked like.

 builtin/log.c   | 1 +
 builtin/notes.c | 3 ++-
 log-tree.c      | 1 +
 pretty.c        | 8 +++++---
 pretty.h        | 1 +
 revision.c      | 4 ++++
 revision.h      | 2 ++
 7 files changed, 16 insertions(+), 4 deletions(-)

diff --git c/builtin/log.c w/builtin/log.c
index b450cd3bde..dc2d7d3a07 100644
--- c/builtin/log.c
+++ w/builtin/log.c
@@ -2148,6 +2148,7 @@ int cmd_format_patch(int argc,
 	rev.commit_format = CMIT_FMT_EMAIL;
 	rev.encode_email_headers = cfg.log.default_encode_email_headers;
 	rev.expand_tabs_in_log_default = 0;
+	rev.indent_empty_lines = 1;
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
diff --git c/builtin/notes.c w/builtin/notes.c
index a3f433ca4c..b559b39cf0 100644
--- c/builtin/notes.c
+++ w/builtin/notes.c
@@ -167,7 +167,8 @@ static void write_commented_object(int fd, const struct object_id *object)
 	struct strbuf cbuf = STRBUF_INIT;
 
 	/* Invoke "git show --stat --no-notes $object" */
-	strvec_pushl(&show.args, "show", "--stat", "--no-notes",
+	strvec_pushl(&show.args, "show", "--stat",
+		     "--no-indent-empty-lines", "--no-notes",
 		     oid_to_hex(object), NULL);
 	show.no_stdin = 1;
 	show.out = -1;
diff --git c/log-tree.c w/log-tree.c
index 1d05dc1c70..e8ea2481ea 100644
--- c/log-tree.c
+++ w/log-tree.c
@@ -879,6 +879,7 @@ void show_log(struct rev_info *opt)
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
 	ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
+	ctx.indent_empty_lines = opt->indent_empty_lines;
 	ctx.output_encoding = get_log_output_encoding();
 	ctx.rev = opt;
 	if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
diff --git c/pretty.c w/pretty.c
index 0bc8ad8a9a..4974af4d02 100644
--- c/pretty.c
+++ w/pretty.c
@@ -2247,13 +2247,14 @@ void pp_remainder(struct pretty_print_context *pp,
 
 	for (;;) {
 		const char *line = *msg_p;
+		int is_blank = 0;
 		int linelen = get_one_line(line);
 		*msg_p += linelen;
 
 		if (!linelen)
 			break;
-
-		if (is_blank_line(line, &linelen)) {
+		is_blank = is_blank_line(line, &linelen);
+		if (is_blank) {
 			if (first)
 				continue;
 			if (pp->fmt == CMIT_FMT_SHORT)
@@ -2262,7 +2263,8 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent)
+
+		if (indent && (!is_blank || pp->indent_empty_lines))
 			pp_handle_indent(pp, sb, indent, line, linelen);
 		else if (pp->expand_tabs_in_log)
 			strbuf_add_tabexpand(sb, opt, pp->color,
diff --git c/pretty.h w/pretty.h
index df267afe4a..21e584e185 100644
--- c/pretty.h
+++ w/pretty.h
@@ -40,6 +40,7 @@ struct pretty_print_context {
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
 	int expand_tabs_in_log;
+	int indent_empty_lines;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git c/revision.c w/revision.c
index 2c36a9c179..ae6b98e701 100644
--- c/revision.c
+++ w/revision.c
@@ -2564,6 +2564,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(optarg, revs);
+	} else if (!strcmp(arg, "--no-indent-empty-lines")) {
+		revs->indent_empty_lines = 0;
+	} else if (!strcmp(arg, "--indent-empty-lines")) {
+		revs->indent_empty_lines = 1;
 	} else if (!strcmp(arg, "--expand-tabs")) {
 		revs->expand_tabs_in_log = 8;
 	} else if (!strcmp(arg, "--no-expand-tabs")) {
diff --git c/revision.h w/revision.h
index 6d369cdad6..49b123387f 100644
--- c/revision.h
+++ w/revision.h
@@ -278,6 +278,7 @@ struct rev_info {
 	struct date_mode date_mode;
 	int		expand_tabs_in_log; /* unset if negative */
 	int		expand_tabs_in_log_default;
+	int		indent_empty_lines;
 
 	unsigned int	abbrev;
 	enum cmit_fmt	commit_format;
@@ -412,6 +413,7 @@ struct rev_info {
 	.expand_tabs_in_log = -1, \
 	.commit_format = CMIT_FMT_DEFAULT, \
 	.expand_tabs_in_log_default = 8, \
+	.indent_empty_lines = 1, \
 }
 
 /**

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

[Phillip Wood]

Hi Junio

On 27/05/2025 18:18, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> 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 for finding that - TIL. I agree with your interpretation that the 
original is in fact safe.

Thanks

Phillip

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

[D. Ben Knoble]

On Tue, May 27, 2025 at 1:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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?

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

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

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

[Eric Sunshine]

On Tue, Jun 3, 2025 at 4:37 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:
> On Tue, May 27, 2025 at 1:31 PM Junio C Hamano <gitster@pobox.com> wrote:
> > 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?
>
> > 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.

For what it's worth, there was a previous attempt at something along
these lines after which a discussion ensued.

https://lore.kernel.org/git/20210830072118.91921-4-sunshine@sunshineco.com/T/

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

[Kristoffer Haugsbakk]

On Tue, Jun 3, 2025, at 22:46, Eric Sunshine wrote:
>> I think this suggestion would also help folks who "git commit -v,"
>> which IIRC is also indented in the template.
>
> For what it's worth, there was a previous attempt at something along
> these lines after which a discussion ensued.
>
> https://lore.kernel.org/git/20210830072118.91921-4-sunshine@sunshineco.com/T/

That’s very relevant, thanks.  It’s not something that I would have
noticed since I am too afraid of editing any file with patch contents in
them.  In part that’s why I find it so convenient that you can send
notes along with patches.

-- 
Kristoffer

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

[Kristoffer Haugsbakk]

On Tue, Jun 3, 2025, at 22:37, D. Ben Knoble wrote:
>> 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?

Yes exactly.  Ain’t nothing more to it than that.  :)
>
>>
>> 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.

In my testing though it doesn’t introduce trailing whitespace.

-- 
Kristoffer Haugsbakk

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

[D. Ben Knoble]

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

On Mon, Jun 9, 2025 at 5:13 PM Kristoffer Haugsbakk
<kristofferhaugsbakk@fastmail.com> wrote:
>
> On Tue, Jun 3, 2025, at 22:37, D. Ben Knoble wrote:
> >> 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?
>
> Yes exactly.  Ain’t nothing more to it than that.  :)
> >
> >>
> >> 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.
>
> In my testing though it doesn’t introduce trailing whitespace.

Really? When committing a recent patch (editor template attached) with
-v (for testing this), I saw trailing whitespace on the blank line
under the added feed.

>
> --
> Kristoffer Haugsbakk



-- 
D. Ben Knoble

[-- Attachment #2: commit-v.txt --]
[-- Type: text/plain, Size: 981 bytes --]

news: add new feed
; Veuillez saisir le message de validation pour vos modifications. Les lignes
; commençant par ';' seront ignorées, et un message vide abandonne la validation.
;
; Sur la branche master
; Votre branche est à jour avec 'origin/master'.
;
; Modifications qui seront validées :
;	modifié :         links/newsboat/urls
;
; Fichiers non suivis:
;	links/bin/git-stack
;
; ------------------------ >8 ------------------------
; Ne touchez pas à la ligne ci-dessus.
; Tout ce qui suit sera éliminé.
diff --git c/links/newsboat/urls i/links/newsboat/urls
index 522d955d..d192de8b 100644
--- c/links/newsboat/urls
+++ i/links/newsboat/urls
@@ -21,6 +21,7 @@ https://ln.ht/_/feed/~ddevault "tech"
 https://maia.crimew.gay/feed.xml "tech"
 https://www.felienne.com/feed "tech"
 https://ronjeffries.com/feed.xml "tech"
+https://eev.ee/feeds/rss.xml "tech"
 
 http://journal.stuffwithstuff.com/rss.xml "~Bob Nystrom" "tech"
 https://blog.sanctum.geek.nz/feed/ "tech"

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

[Junio C Hamano]

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

> Really? When committing a recent patch (editor template attached) with
> -v (for testing this), I saw trailing whitespace on the blank line
> under the added feed.

True.  Unless you use a special configuration, which might break
other people's "patch" implementations (I think "git apply" has
learned to grok this ages ago).

$ git -c diff.suppressBlankEmpty=yes commit -v