Skip to content

Conversation

@ascholerChemeketa
Copy link
Contributor

The ToC is a significant chunk of each HTML page and is mostly the same on every page. This PR precomputes the ToC into a variable and then on each page augments that with the necessary active and contains-active classes.

Reduces xsltproc time by ~15-20% for sample book. For books with lots of smaller pages savings can be 35%+.

Clean diffs on sample book.

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 21, 2024

That is brilliant! Cleaner code, no-change diffs, and a speed-up. (And a nice sequence of commits.)

Will update website shortly, a few more PRs to do. Keep 'em coming!

@rbeezer rbeezer closed this Dec 21, 2024
@oscarlevin
Copy link
Member

Uh-oh.... this seems to have broken revealjs builds. I get a lot of:

error: * PTX:BUG:   an element ("frontmatter") does not know its level
error: * PTX:BUG:     asking if a "frontmatter" is a leaf, for a document that is not a "book" nor an "article"

starting at the commit that these were merged.

@ascholerChemeketa
Copy link
Contributor Author

ascholerChemeketa commented Dec 23, 2024

It looks like it still generates the show correctly, but it does produce those scary-looking messages.

They all come from where the ToC is being built into a variable.

The quick and dirty fix is to add a variable that pretext-revealjs.xsl sets and pretext-html checks while building the ToC. That may be part of the right fix as well - there is no reason that pretext-revealjs.xsl needs the ToC generated. It does feel a little ugly to have the more generic sheet (pretext-html) rely on hints from the most specific sheet, but that may be unavoidable.

It does feel like there is a bigger issue here. Shouldn't we be able to provide the answer to "is-leaf" and "level" for "frontmatter" in a slideshow? If that was fixed the bug reports would go away. That seems like it has implications for #2334

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 23, 2024

Reviewed the PR and the RevealJs stylesheet. Not seeing it.

Shouldn't we be able to provide the answer to "is-leaf" and "level" for "frontmatter" in a slideshow?

Might just be a matter of defining a level for slideshow? And/or slideshow/section.

I'll get back to this in a bit, when I am able.

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 23, 2024

there is no reason that pretext-revealjs.xsl needs the ToC generated

So we override the toc-cache-rtf global variable in the derived worksheet. On it. Push in a minute.

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 23, 2024

Fix in at: 2fb780c

Thanks @oscarlevin for the heads-up and @ascholerChemeketa for the hints.

@oscarlevin
Copy link
Member

Can confirm that the most recent commit fixed the issue. Thanks!

@ascholerChemeketa ascholerChemeketa deleted the HTML-toc-single-build branch January 15, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants