Hi folks, At 2023-04-05T18:07:28-0500, G. Branden Robinson wrote: > At 2023-04-05T13:04:45-0500, Dave Kemper wrote: > > The missing page break you're seeing here is due to a combination of > > factors: > > - Immediately before the .bp, you call the .DE macro > > - The .DE macro was recently modified to invoke the .ns request. > > - When no-space mode (.ns) is in effect, a .bp is ignored. (This > > is long-established roff behavior.) > > > > The .ns request was added to .DE to address a rendering issue with > > multiple displays in a row. (See > > http://savannah.gnu.org/bugs/?62688 for the full story.) Your case > > appears to be an unintended side effect. > > Yes, this is a full-on bug in groff Git, and it's my fault. I've > filed https://savannah.gnu.org/bugs/?64005 about it. > > It is a regression from 1.22.4, so I will work on it right away, and > hope to have it in place by this weekend so Bertrand can consider it > for 1.23.0.rc4.
I've attached what I have in my working copy. I'd appreciate reviews for correctness. It passes the regression test I wrote for it. Here's a dissection of the change to s.tmac, beyond what is covered in the ChangeLog. 1. No-space mode (register \n[.ns]) is saved. 2. No-space mode is (perhaps redundantly) cancelled. 3. Invoke the aliased `bp` request with the control character corresponding to our own call. 4. Invoke the aliased `bp` with the argument we were given, if any. N.B. \\$@ is not usable here, because it quotes arguments, and a formatter request, not being a macro, does not handle quotation as macros do. 5. Reënable no-space mode only if we were in it when called. 6. Discard the register used to save no-space mode enablement. 7. Replace all invocations of `bp` already present with invocations of its new alias instead, to preserve existing behavior precisely. Feedback welcome. Regards, Branden
commit 3b380601ceed46c7be26616fcc2dc942e8b2709c
Author: G. Branden Robinson <g.branden.robin...@gmail.com>
Date: Wed Apr 5 19:08:03 2023 -0500
[ms]: Regression-test Savannah #64005.
* tmac/tests/s_honor-page-break-after-display.sh: Do it.
* tmac/tmac.am (tmac_TESTS): Run test.
Test fails at this commit.
diff --git a/ChangeLog b/ChangeLog
index 704ac33a6..944a80402 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-04-05 G. Branden Robinson <g.branden.robin...@gmail.com>
+
+ [ms]: Regression-test Savannah #64005.
+
+ * tmac/tests/s_honor-page-break-after-display.sh: Do it.
+ * tmac/tmac.am (tmac_TESTS): Run test.
+
2023-03-24 G. Branden Robinson <g.branden.robin...@gmail.com>
* tmac/mdoc/doc-common (Sh): Restore hyphenation configured with
diff --git a/tmac/tests/s_honor-page-break-after-display.sh b/tmac/tests/s_honor-page-break-after-display.sh
new file mode 100755
index 000000000..c50918c86
--- /dev/null
+++ b/tmac/tests/s_honor-page-break-after-display.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+#
+# Copyright (C) 2023 Free Software Foundation, Inc.
+#
+# This file is part of groff.
+#
+# groff is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# groff is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+# for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+groff="${abs_top_builddir:-.}/test-groff"
+
+# Regression-test Savannah #64005.
+
+input='.pl 18v
+.LP
+The first page is \n%.
+.DS
+display
+.DE
+.bp
+.LP
+The second page is \n%.
+.pl \n(nlu'
+
+output=$(printf '%s\n' "$input" | "$groff" -Tascii -P-cbou -ms)
+echo "$output"
+echo "$output" | grep -Fqx 'The second page is 2.'
+
+# vim:set ai et sw=4 ts=4 tw=72:
diff --git a/tmac/tmac.am b/tmac/tmac.am
index d7f67d2f6..f0c2d9bf0 100644
--- a/tmac/tmac.am
+++ b/tmac/tmac.am
@@ -230,6 +230,7 @@ tmac_TESTS = \
tmac/tests/s_TC-works-with-percent-in-custom-titles.sh \
tmac/tests/s_XA-literal-no-argument-suppresses-leader.sh \
tmac/tests/s_honor-MINGW-when-two-columns.sh \
+ tmac/tests/s_honor-page-break-after-display.sh \
tmac/tests/s_mark-column-start-correctly.sh \
tmac/tests/s_no-excess-space-around-displays.sh \
tmac/tests/s_rejects-too-short-page-lengths.sh
commit e7eeafd7cfa18a2fbe85621142ff40731882a224
Author: G. Branden Robinson <g.branden.robin...@gmail.com>
Date: Wed Apr 5 20:04:17 2023 -0500
[ms]: Fix Savannah #64005.
* tmac/s.tmac (@break-page, bp): Define alias for `bp` request and
wrapper for `bp` to (if needed) temporarily disable no-space mode, so
that a document's `bp` requests are honored even if no-space mode is
on, as can happen after displays. Fixes a regression from groff
1.22.4 and historical ms implementations introduced by me on 6 July
when resolving Savannah #62688.
Fixes <https://savannah.gnu.org/bugs/?64005>. Thanks to Michał
Kruszewski for reporting the problem and Dave Kemper for identifying the
cause.
ANNOUNCE: Acknowledge Michał.
diff --git a/ANNOUNCE b/ANNOUNCE
index 5a1137b13..1c5ea30ee 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -207,6 +207,7 @@ John Gardner
KUBO Koichi
Keith Marshall
Ken Mandelberg
+Michał Kruszewski
Nikita Ivanov
Oliver Corff
Olle Lögdahl
diff --git a/ChangeLog b/ChangeLog
index 944a80402..07531e697 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2023-04-05 G. Branden Robinson <g.branden.robin...@gmail.com>
+
+ [ms]: Fix Savannah #64005.
+
+ * tmac/s.tmac (@break-page, bp): Define alias for `bp` request
+ and wrapper for `bp` to (if needed) temporarily disable no-space
+ mode, so that a document's `bp` requests are honored even if
+ no-space mode is on, as can happen after displays. Fixes a
+ regression from groff 1.22.4 and historical ms implementations
+ introduced by me on 6 July when resolving Savannah #62688.
+
+ Fixes <https://savannah.gnu.org/bugs/?64005>. Thanks to Michał
+ Kruszewski for reporting the problem and Dave Kemper for
+ identifying the cause.
+
2023-04-05 G. Branden Robinson <g.branden.robin...@gmail.com>
[ms]: Regression-test Savannah #64005.
diff --git a/tmac/s.tmac b/tmac/s.tmac
index bdf36b885..343b5024a 100644
--- a/tmac/s.tmac
+++ b/tmac/s.tmac
@@ -95,6 +95,21 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
. @divert \\$*
..
.
+.\" Wrap the `bp` request so that we disregard the enablement of
+.\" no-space mode, which we enter after displays and equations to
+.\" prevent multiple sources of vertical spacing (like the DD and PD
+.\" registers) from incorrectly accumulating. Thus, if the user
+.\" requests a page break, we honor it. See Savannah #62688 & #64005.
+.als @break-page bp
+.de bp
+. nr @saved-no-space-mode \\n[.ns]
+. rs
+. if \\n[.br] '@break-page \\$1
+. el .@break-page \\$1
+. if \\n[@saved-no-space-mode] .ns
+. rr @saved-no-space-mode
+..
+.
.de @init
.if !rPO .nr PO \\n(.o
.\" a non-empty environment
@@ -373,8 +388,8 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
.AE
..
.de cov*break-page
-.ie \\n[cov*rp-no-renumber] .bp
-.el .bp 1
+.ie \\n[cov*rp-no-renumber] .@break-page
+.el .@break-page 1
..
.de cov*print
.als cov*print @nop
@@ -559,7 +574,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
. \" flush out any floating keeps
. while \\n[kp@tail]>\\n[kp@head] \{\
. rs
-. bp
+. @break-page
. \}
.\}
.ie !\\n(.$ \{\
@@ -650,7 +665,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
. \" Switching environments ensures that we don't get an unnecessary
. \" blank line at the top of the page.
. ev ne
-' bp
+' @break-page
. ev
.\}
.el \{\
@@ -665,7 +680,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
. rm pg*next-format
. \}
. \}
-' bp
+' @break-page
.\}
..
.\" pg@begin number format
@@ -698,9 +713,9 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
.\" overflow left, or floating keeps.
.while \\n[kp@tail]>\\n[kp@head]:\\n[pg@fn-flag] \{\
. rs
-. bp
+. @break-page
.\}
-.bp
+.@break-page
..
.nr pg@text-ended 0
.de pg@end-text
signature.asc
Description: PGP signature
