Skip to content
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

LaTeX support for argmin-testfunctions docs #530

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

airwoodix
Copy link

@airwoodix airwoodix commented Oct 21, 2024

This patch adds LaTeX/KaTeX support to the argmin-testfunctions crate documentation.

The HTML code in argmin-testfunctions/katex-header.html is added to the header of all the documentation pages in that crate and loads the KaTeX auto-render extension from the recommended CDN. It may be possible to serve the KaTeX code from local files but I didn't investigate this.

As an example, the documentation of a couple of test function is migrated to using KaTeX rendering. For example that of the ackley function:

image

If it is acceptable, the migration of other documentation pages shall be done in subsequent PRs.

As a fly-by, I added some missing cross links and fixed some typos. This patch doesn't contain any Rust code changes.

Resolves #418.

Local build

The workspace's top-level Cargo configuration sets the RUSTDOCFLAGS to include the HTML header in all docs pages. This enables KaTeX support for the documentation of all crates for local builds. This is misleading because there's no support in the docs.rs build for crates other than argmin-testfunctions (see below). Unfortunately, Cargo does not read config files from crates within the workspace so there's no easy way to restrict that configuration to a single crate while keeping the unified workspace build.

docs.rs build

The docs.rs build was tested on a docs.rs development build. The docs.rs build is configured through data in the package.metadata.docs.rs table in the crate's manifest. Workspace metadata is not supported. Also, the referenced files must be part of the crate, so enabling support for other crates would probably require duplicating the katex-header.html file to the other crates, which is a bit of a maintenance burden.

Error handling

Syntax errors in the KaTeX code unfortunately only lead to broken rendering. Since the KaTeX code is interpreted in the browser, it may be quite involved to automatically propagate errors to the rustdoc call.

Reference

https://github.com/victe/rust-latex-doc-minimal-example (and many other publicly available crates)

@codecov-commenter
Copy link

codecov-commenter commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.49%. Comparing base (be2444e) to head (b7eb0c2).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #530   +/-   ##
=======================================
  Coverage   91.49%   91.49%           
=======================================
  Files         178      177    -1     
  Lines       23623    23621    -2     
=======================================
- Hits        21613    21612    -1     
+ Misses       2010     2009    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@airwoodix
Copy link
Author

Setting build.rustdocflags in the workspace root's Cargo configuration file breaks the book doctests workflow, because the path to katex-header.html doesn't resolve from media/book/tests. As a workaround, I added an override in the workflow step, such that the RUSTDOCFLAGS are empty when running the book doctests.

There's a similar issue with a similar workaround for the argmin-math tests (and anything that is not invoked with the workspace root as working directory).

Open point: is it preferable to set RUSTDOCFLAGS='--html-in-header ...' only during the actual docs build (instead of the global setting in .cargo/config.toml)?

@stefan-k
Copy link
Member

stefan-k commented Oct 27, 2024

Firstly, this is an absolutely magnificent PR, both in terms of work done and description! Thank you for the massive effort you have taken to make this work!

Do I understand it correctly that the main problem is that both docs.rs and local builds have to be configured/set up separately? If that is the case, I think I can live with it.

Open point: is it preferable to set RUSTDOCFLAGS='--html-in-header ...' only during the actual docs build (instead of the global setting in .cargo/config.toml)?

Would this fix the discrepancy between docs.rs and local builds, or would this avoid the work arounds for argmin-math and media/book/tests (and instead requires setting RUSTDOCSFLAGS for argmin-testfunctions)?

The failing clippy lints should be fixed in main.

@airwoodix airwoodix force-pushed the resolve-418-docs-latex branch from b7eb0c2 to 966c27d Compare October 28, 2024 21:27
@airwoodix
Copy link
Author

Thanks for the feedback!

Do I understand it correctly that the main problem is that both docs.rs and local builds have to be configured/set up separately?

Yes, that's correct.

Would this fix the discrepancy between docs.rs and local builds, or would this avoid the work arounds for argmin-math and media/book/tests (and instead requires setting RUSTDOCSFLAGS for argmin-testfunctions)?

It would remove the workarounds for media/book/tests and argmin-math. Setting RUSTDOCFLAGS for the CI step would work and remove the new .cargo/config.toml and the mentioned workarounds:

diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index bbd3a70c..affe0c1f 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -19,6 +19,8 @@ jobs:
           components: rustfmt,rust-src

       - name: Build documentation
+        env:
+          RUSTDOCFLAGS: "--html-in-header ./crates/argmin-testfunctions/katex-header.html"
         run: cargo doc --features "argmin-math/latest_all,argmin/full" --no-deps --workspace --exclude "example-*"

       - name: Forwarding

Caveats:

  • building locally during development is more cumbersome (one also needs to set RUSTDOCFLAGS in the local environment)
  • the HTML header is again applied to all the crates, not just to argmin-testfunctions. I'm not aware of a way to achieve the latter while keeping the single doc build command issued from the workspace root.

The failing clippy lints should be fixed in main.

Thanks! I rebased the branch on main.

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.

Add LaTeX support to docs
3 participants