* [PATCH] pretty: fix lost indent after newline in %w()
@ 2026-10-14 17:23 Andy Koppe
2026-10-15 20:11 ` Andy Koppe
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andy Koppe @ 2026-10-14 17:23 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Johannes Schindelin, Andy Koppe
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. 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.".
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.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
t/t4202-log.sh | 6 ++++++
t/t4205-log-pretty-formats.sh | 2 +-
utf8.c | 4 +++-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 06552a6271..5895780e8f 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -94,6 +94,12 @@ test_expect_success 'format %w(,1,2)' '
test_cmp expect actual
'
+test_expect_success 'format %w(11)' '
+
+ git log -2 --format="%w(11) This is%n the %s%n commit." > actual &&
+ test_cmp expect actual
+'
+
cat > expect << EOF
$(git rev-parse --short :/sixth ) sixth
$(git rev-parse --short :/fifth ) fifth
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 4be5c51489..0733f22d58 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1139,7 +1139,7 @@ test_expect_success 'log --pretty with invalid padding format' '
test_expect_success 'log --pretty with magical wrapping directives' '
commit_id=$(git commit-tree HEAD^{tree} -m "describe me") &&
git tag describe-me $commit_id &&
- printf "\n(tag:\ndescribe-me)%%+w(2)" >expect &&
+ printf "\n\n(tag:\ndescribe-me)%%+w(2)" >expect &&
git log -1 --pretty="format:%w(1)%+d%+w(2)" $commit_id >actual &&
test_cmp expect actual
'
diff --git a/utf8.c b/utf8.c
index 96460cc414..bb55bf357f 100644
--- a/utf8.c
+++ b/utf8.c
@@ -322,6 +322,7 @@ void strbuf_add_wrapped_text(struct strbuf *buf,
space++;
if (*space == '\n') {
strbuf_addch(buf, '\n');
+ space++;
goto new_line;
}
else if (!isalnum(*space))
@@ -333,9 +334,10 @@ void strbuf_add_wrapped_text(struct strbuf *buf,
text++;
}
else {
+ space += isspace(*space);
new_line:
strbuf_addch(buf, '\n');
- text = bol = space + isspace(*space);
+ text = bol = space;
space = NULL;
w = indent = indent2;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] pretty: fix lost indent after newline in %w()
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
2026-10-17 8:11 ` [PATCH v2] " Andy Koppe
2 siblings, 0 replies; 6+ messages in thread
From: Andy Koppe @ 2026-10-15 20:11 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Johannes Schindelin
Andy Koppe wrote:
> 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.".
Admittedly, that's a bit of a silly example and test, just to keep it
small. My actual use case is "%w(72)%B", because I often have to deal
with repos that don't stick to line length standards, which can make a
visual mess with --graph or with wide terminal windows.
As a better example, lists of files and their changes like this ...
- foo.c
Frobble.
- bar.c
Bobble.
... lose a space of indentation, thus breaking alignment:
- foo.c
Frobble.
- bar.c
Bobble.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pretty: fix lost indent after newline in %w()
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
2026-10-15 20:59 ` Junio C Hamano
2026-10-17 8:26 ` Andy Koppe
2026-10-17 8:11 ` [PATCH v2] " Andy Koppe
2 siblings, 2 replies; 6+ messages in thread
From: René Scharfe @ 2026-10-15 20:42 UTC (permalink / raw)
To: Andy Koppe, git; +Cc: Johannes Schindelin
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é
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pretty: fix lost indent after newline in %w()
2026-10-15 20:42 ` René Scharfe
@ 2026-10-15 20:59 ` Junio C Hamano
2026-10-17 8:26 ` Andy Koppe
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2026-10-15 20:59 UTC (permalink / raw)
To: René Scharfe; +Cc: Andy Koppe, git, Johannes Schindelin
René Scharfe <l.s.r@web.de> writes:
> 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..
Yeah, to me, it sounds more productive use of our time and a good
direction.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pretty: fix lost indent after newline in %w()
2026-10-15 20:42 ` René Scharfe
2026-10-15 20:59 ` Junio C Hamano
@ 2026-10-17 8:26 ` Andy Koppe
1 sibling, 0 replies; 6+ messages in thread
From: Andy Koppe @ 2026-10-17 8:26 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Johannes Schindelin
On Thu, 15 Oct 2026 at 21:42, René Scharfe <l.s.r@web.de> wrote:
> 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"
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.
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] pretty: fix lost indent after newline in %w()
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
@ 2026-10-17 8:11 ` Andy Koppe
2 siblings, 0 replies; 6+ messages in thread
From: Andy Koppe @ 2026-10-17 8:11 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Johannes Schindelin, Andy Koppe
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. 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. This affected
things like code examples or multi-line list items starting with "- "
and continuing with two spaces of indentation.
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.
Document the %w placeholder properly, removing the reference to `git
shortlog -w`, which has different defaults and isn't concerned with
paragraph reflowing.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/pretty-formats.adoc | 9 +++++++--
t/t4202-log.sh | 6 ++++++
t/t4205-log-pretty-formats.sh | 2 +-
utf8.c | 4 +++-
4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/Documentation/pretty-formats.adoc b/Documentation/pretty-formats.adoc
index 2ae0eb11a9..4fb0419715 100644
--- a/Documentation/pretty-formats.adoc
+++ b/Documentation/pretty-formats.adoc
@@ -169,8 +169,13 @@ The placeholders are:
on the next placeholders until the color is switched
again.
++%m++:: left (`<`), right (`>`) or boundary (`-`) mark
-++%w(++`[<w>[,<i1>[,<i2>]]]`++)++:: switch line wrapping, like the `-w` option of
- linkgit:git-shortlog[1].
+++%w(++`[<w>[,<i1>[,<i2>]]]`++)++:: set line wrapping and indentation.
+ Output is word-wrapped at _<w>_ columns, with the first line
+ indented by _<i1>_ spaces and subsequent lines indented by
+ _<i2>_ spaces. A width of 0 disables wrapping. Omitted values
+ default to 0, so ++%w()++ turns off both wrapping and indentation.
+ Newlines that are followed by an ASCII letter or digit are replaced
+ by spaces to reflow paragraphs. Existing indentation is kept.
++%<(++`<n>[,(trunc|ltrunc|mtrunc)]`++)++:: make the next placeholder take at
least N column widths, padding spaces on
the right if necessary. Optionally
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 06552a6271..5895780e8f 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -94,6 +94,12 @@ test_expect_success 'format %w(,1,2)' '
test_cmp expect actual
'
+test_expect_success 'format %w(11)' '
+
+ git log -2 --format="%w(11) This is%n the %s%n commit." > actual &&
+ test_cmp expect actual
+'
+
cat > expect << EOF
$(git rev-parse --short :/sixth ) sixth
$(git rev-parse --short :/fifth ) fifth
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 4be5c51489..0733f22d58 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1139,7 +1139,7 @@ test_expect_success 'log --pretty with invalid padding format' '
test_expect_success 'log --pretty with magical wrapping directives' '
commit_id=$(git commit-tree HEAD^{tree} -m "describe me") &&
git tag describe-me $commit_id &&
- printf "\n(tag:\ndescribe-me)%%+w(2)" >expect &&
+ printf "\n\n(tag:\ndescribe-me)%%+w(2)" >expect &&
git log -1 --pretty="format:%w(1)%+d%+w(2)" $commit_id >actual &&
test_cmp expect actual
'
diff --git a/utf8.c b/utf8.c
index 96460cc414..bb55bf357f 100644
--- a/utf8.c
+++ b/utf8.c
@@ -322,6 +322,7 @@ void strbuf_add_wrapped_text(struct strbuf *buf,
space++;
if (*space == '\n') {
strbuf_addch(buf, '\n');
+ space++;
goto new_line;
}
else if (!isalnum(*space))
@@ -333,9 +334,10 @@ void strbuf_add_wrapped_text(struct strbuf *buf,
text++;
}
else {
+ space += isspace(*space);
new_line:
strbuf_addch(buf, '\n');
- text = bol = space + isspace(*space);
+ text = bol = space;
space = NULL;
w = indent = indent2;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-17 8:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-10-15 20:59 ` Junio C Hamano
2026-10-17 8:26 ` Andy Koppe
2026-10-17 8:11 ` [PATCH v2] " Andy Koppe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox