From: "René Scharfe" <l.s.r@web.de>
To: Andy Koppe <andy.koppe@gmail.com>, git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] pretty: fix lost indent after newline in %w()
Date: Thu, 15 Oct 2026 22:42:28 +0200 [thread overview]
Message-ID: <fc0dabfb-3dda-4b12-a1fb-184d1e553623@web.de> (raw)
In-Reply-To: <20261014172333.2224123-1-andy.koppe@gmail.com>
On 10/14/26 7:23 PM, Andy Koppe wrote:
> 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"
> 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"
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.
> 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.
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..
René
next prev parent reply other threads:[~2026-10-15 20:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-14 17:23 [PATCH] pretty: fix lost indent after newline in %w() Andy Koppe
2026-10-15 20:11 ` Andy Koppe
2026-10-15 20:42 ` René Scharfe [this message]
2026-10-15 20:59 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc0dabfb-3dda-4b12-a1fb-184d1e553623@web.de \
--to=l.s.r@web.de \
--cc=Johannes.Schindelin@gmx.de \
--cc=andy.koppe@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox