New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[styles] Improve automatic page breaking #1764
Conversation
source/layout.tex
Outdated
@@ -58,3 +58,6 @@ | |||
|
|||
% Leave more room for section numbers in TOC | |||
\cftsetindents{section}{1.5em}{3.0em} | |||
|
|||
% Allow for substantial empty space at the bottom of a page. | |||
\addtolength{\topskip}{0pt plus 25pt} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we perhaps look for a memoir way of doing this, rather than modifying the lengths directly? I worry about hard-to-trace interactions if we go behind memoir's back like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reviewed the memoir documentation, section 2 "Laying out the page", and some of the calculations do consider \topskip (called "t" there), but there is no other explicit mention of \topskip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There doesn't seem to be a memoir way of doing this, and we need it when the page otherwise has no stretchable vertical space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Section 3.5 of the memoir documentation might be helpful (e.g., the memoir-specific \sloppybottom
macro and discussion surrounding it).
@tkoeppe: Hm... That's a heading, a \libraryindex and a codeblock. Nothing that I directly touched, so this must be a fallout of the \topskip change. Plus, headings should really stick with whatever follows, so this is rather mysterious to me. @godbyk , @tkoeppe: Does any of you know a way to show in the Latex log the result badness (= sum of penalties / demerits) of each page break? That would allow us to find out whether this particular page break is a case of "several infinitely bad options". |
After some experimentation, that orphaned heading for [char.traits.specializations.char] is a myth to me. Going to as much as "\addtolength{\topskip}{0pt plus 35pt}" (note 35pt vs. 25pt) doesn't make that go away, although we should now have given enough allowance to move the heading to the next page. |
This stuff in styles.tex looks suspicious:
"plus -1ex" seems broken; we're defining stretchable space with "plus" and I fully expect a positive number afterwards. |
Ah, the negative numbers "prevent indentation of the line that follows". But we never indent anything (except explicitly via an environment), so this is totally pointless and confusing. |
I suspect the negative sign there is a legacy from before we fully turned off memoir's "indent the first line of each paragraph" behavior. If there are no visual diffs, just remove it :) |
Conclusion: [char.traits.specializations.char] will need a \newpage in the final preparations for C++20, unless we can find out why the page break happens there. Still much better than the tens of \newpages we needed for C++17. |
While we're considering such issues, is there some way we could discourage linebreaks within a codeblock / itemdecl environment, except after semicolons or at blank lines? A lot of the C++17 vertical whitespace fixes I applied were fixing that kind of issue. |
@zygoloid: And I'd also want a "magic blank line" feature that then doesn't emit the blank line at the top of the next page! But that seems fairly advanced. |
@zygoloid: "discourage linebreak"? I guess you mean "pagebreaks". I have found zero mention of "page break" in the listings documentation. That said, it seems with my changes (in particular the added stretchability), we get far less / close to none pagebreaks within a short itemdecl anyway, which goes a long way. |
@jensmaurer You can use
|
@godbyk: Thanks a lot, that's cool!
It appears that a "listings" environment has a page break penalty of -50 before and after it, which is (in principle) a good idea: Breaking before a listing is certainly better than breaking in the middle of it. However, that somehow seems to override / cancel the general setting "do not break after a heading". |
Ok, I've now had a look at the source of the "listings" package. It has a non-configurable page break penalty of -50 before and after each (inline) listing environment. Which, absent other changes, totally runs counter to the idea of keeping an itemdecl together with the itemdescr, and keeping a heading with any listing that might immediately follow. The correct approach is to make these penalties configurable (and/or conditionally absent), similar to "beginpenalty" / "endpenalty" in the enumitem package. |
Update on progress: As an experiment, I've removed listings' \penalty-50, expecting that the "regular" penalties would kick in (i.e. plain page break opportunity with penalty=0). That nearly works as expected, except that there's a \nobreak somewhere in |
Here is what happens:
Now, I have to have an even deeper look at the listings package to find out where it kills \everypar without resetting \if@nobreak. |
Ok, the listings package and \everypar need to get a divorce. |
I've updated this. The bad macros from the listings package have been redefined / overridden in styles.tex. |
@jensmaurer There are a bunch of hooks provided by the |
I've been chatting to one of the maintainers of the listings package. One possible option would be to set the EveryPar hook to perform the actions that memoir's section command sets
|
@godbyk: I've looked at the hooks, and they don't seem to help avoiding / configuring \everypar or \par\penalty. Please see the large green diff in styles.tex for the specific changes vs. the stock package, each marked by a TeX comment. To summarize the issue with \everypar: The section heading sets \everypar to "something", but the listings package cavalierly ignores the previous value of \everypar and sets its own value. That's not good. (LaTeX doesn't seem to have infrastructure for an extensible and flexible mechanism for \everypar that could be used by several clients independently. Poster-child global variable problem, I'd say.) |
@zygoloid: Ok, that's one option. Except that there's only "add-to-hook", not "remove-from-hook" to perform the \everypar reset. (Nothing an additional level of indirection can't fix.) However, that approach means copying some details of "memoir" into our code, which is probably as bad as copying some details of "listings" into our code. And we need to massage "listings" even more to detect empty lines and semicolons and introduce an attractive page break there. |
Here's the next update, with the following additional features:
|
And here's the final update, with the following additional features:
At this time, we still prefer breaking inside an itemdecl (even when not after a semicolon) vs. breaking between the itemdecl and the itemdescr. (See [alg.remove] for an example.) We can fine-tune this by increasing the "midpenalty" in itemdecl. |
If this means that we prefer breaking between two declarations in an itemdecl over breaking between itemdecl and itemdescr, then I think that's the right thing, even if in bad cases we end up breaking between decl and descr. I don't expect this work to remove the need for manual vertical whitespace fixes for C++20, but it sounds like it should at least make them substantially easier, and make the working drafts more readable. How confident are you that this change doesn't introduce significant formatting regressions (very ugly pagebreaks, text outside the bounds of the textblock, etc)? Given that we only have a couple of days before the mailing deadline, I'm trying to determine whether we should merge this now, or merge it after we cut the pre-meeting working draft. @tkoeppe Are you OK with the approach in use here? I'm not happy with copy-pasting chunks of listings, but if only the alternative is copy-pasting chunks of memoir, I don't have a strong preference. (Ideally I'd like to minimize the amount of delta we introduce in this way, though.) @jensmaurer Can you check what the licensing terms on the listings source are? If we're not permitted to copy and modify the code like this, we may need to get more creative... |
@zygoloid Both I haven't compared the copied code to the original yet, but if it's possible to patch the macros without explicitly overwriting them, that's sometimes a better option (as it allows for other packages or future updates of those packages to modify the code without us overwriting their modifications). |
@zygoloid: Quote: % The listings package and its drivers may be distributed and/or modified Since we're not claiming to distribute a modified "listings" package (in fact, you need the original package for my modifications to work), I think we simply make a "Derivative Work" for our own purposes, which is fine. |
@zygoloid: I did a "diffpdf" on the core part (few changes, all for the better --- good testbed for tweaking and finding+fixing a few bugs) and a "finger repeatedly hitting the page down key, eyes focused on the bottom end of the page" review of all 1200 or so substantive pages yesterday, and I noticed these things:
Regarding copy-pasting listings vs. memoir, the changes I did are such that there's no way around copy-pasting parts listings at this time. I've used the provided hooks as much as I could. |
@zygoloid: In short, I'm in favor of merging before the mailing. We might consider sending an e-mail to -all so that people stay alert for bad pagination. |
@godbyk: I agree with the general approach, but that utterly fails if there's a |
@jensmaurer Yes, it's unfortunately not always possible. It's a known problem with LaTeX. Packages are always stomping on each other, redefining the same macros (sometimes politely, sometimes less so), and vying for control over what hooks are available. One thing I would recommend is to use |
@godbyk: I've contacted the "listings" maintainer; maybe the next version of the package already contains the features we need. |
I've done some review of the result of this change too, and I like it a lot. In fact, I've only found one page break I don't like so far: in [util.sharedptr.shared.const]'s itemdecl between p8 and p9, we put the first three declarations on the bottom of one page (with a large section of empty whitespace below them), and put the fourth declaration and the itemdescr at the top of the next page. The net result looks pretty silly. I think it would be preferable to keep the itemdecl together here, and break the 3-line p9 across the page. (Leaving an orphaned single line is bad, but the current formatting is worse.) I wonder if we can increase the penalty for breaking within an itemdecl to cause that to happen. |
@jensmaurer Can you apply @godbyk's suggestion of using |
Add keywords beginpenalty, endpenality, midpenalty, emptylinepenalty, semicolonpenalty. Also avoid redefining \everypar, because it interferes with section headings.
@zygoloid: Breaking a 3-line paragraph means working against a \clubpenalty = windowpenalty = 1000. The current listings midpenalty = 500 doesn't compete here, obviously. |
@zygoloid: Instead of \CheckCommand (which requires lots of cut&paste), I'm now using \lst@CheckVersion and check that the listings package is at exactly v1.6. It's likely I need to adapt my changes in some way for the next revision of the listings package anyway. |
- Strongly discourage breaking between itemdecl and itemdescr. - Mildly discourage breaking inside an itemdecl. - Encourage breaking after a codeblock. - Encourage breaking at empty lines in a codeblock. - Encourage breaking after a semicolon or closing brace. - Mildly discourage breaking before an itemized list. - Allow substantial whitespace at the end of a page.
Strongly discourage breaking between itemdecl and itemdescr.
Mildly discourage breaking before an itemized list.
Add stretchable vertical space between paragraphs and before listings.
Allow substantial whitespace at the end of a page.
Fixes #1752.