Thread (6 messages) 6 messages, 3 authors, 8d ago

Re: [PATCH] pretty: fix lost indent after newline in %w()

flat view

From: Andy Koppe <hidden>
Date: 2026-10-17 08:26:20

On Thu, 15 Oct 2026 at 21:42, René Scharfe [off-list ref] wrote:
On 10/14/26 7:23 PM, Andy Koppe wrote:
quoted
When strbuf_add_wrapped_text() meets a newline followed by a
non-alphanumeric character, it treats it as a hard line break and jumps
to the new_line label.
I assume you're referring to ae0b270230 (print_wrapped_text(): allow
hard newlines, 2007-11-11), which started to replace single newlines
preceding alphanumeric characters with a space, and to keep double
newlines and calling them hard.

E.g. the following format pairs are equivalent:

   - hard newline:                    "%w(9)%n%nA" == "%n%nA"
   - replace newline before alpha:    "%w(9)%nA"   == " A"
   - keep newline before punctuation: "%w(9)%n-A"  == "%n-A"
quoted
That code was written for the case where a line
gets too long. It moves past the space where the line was broken by
setting the next line's start to 'space + isspace(*space)'.

After a hard line break, 'space' already points to the first character
of the next line. If that character is whitespace, it was wrongly
skipped, so the line lost one column of indentation. For example,

  git log --format="%w(11) This is%n  the %s%n  commit."

printed " the ..." and " commit." rather than "  the ..." and
"  commit.".
So currently a newline before whitespace is kept, but that whitespace
gets eaten, and you want to keep it instead:

  - now:   "%w(9)%n A" == "%nA"
  - patch: "%w(9)%n A" == "%n A"
Correct.
The commit message of ae0b270230 doesn't mention this case specifically.
I also get the impression that this consumption of the first whitespace
character was not intended.
I agree.
quoted
Fix this by skipping the space only in the line-too-long case. In the
double-newline case, skip the second newline explicitly, because that
case relied on the old skipping.

Add a test for this, and update the expected output of the "magical
wrapping directives" test in t4205. There, %+d emits a newline followed
by " (tag: ...)", and that leading space is no longer lost. With a wrap
width of 1, the space now causes an extra line break.
This behavior stood for more than 18 years.  Someone might be relying
on it -- this test is one example.  And you can easily work around it
by adding one more space to the format.
True, but that isn't the actual use case. In v2 I've amended the commit message:
"This affected things like code examples or multi-line list items
starting with "- "
and continuing with two spaces of indentation."
On the other hand this hard newline concept is not explained in the
docs as far as I can see.  That makes it fair game for change in my
book (within reason), as long as that doc gap is filled.  My 2ct..
Very good point. I've added dedicated documentation for %w in v2.

Btw, the detection of paragraph continuations by looking for an
alphanumeric ASCII character at the start of the next line could be
improved by looking for quotes and opening parentheses too and by
supporting Unicode. But I haven't attempted that here.

Andy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help