argmin
argmin copied to clipboard
LaTeX support for argmin-testfunctions docs
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:
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 Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 92.10%. Comparing base (
98fa6a3) to head (e03140d).
Additional details and impacted files
@@ Coverage Diff @@
## main #530 +/- ##
==========================================
- Coverage 92.10% 92.10% -0.01%
==========================================
Files 177 177
Lines 23675 23675
==========================================
- Hits 21807 21806 -1
- Misses 1868 1869 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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)?
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.
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
RUSTDOCFLAGSin 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.
Apologies once again for the long wait!
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.
This is fine with me.
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.
Caveat 2 seems fine with me, but caveat 1 does sound cumbersome and it would be nice if this could be avoided.
Unfortunately I lost track a bit of this PR (apologies again, because I can't really express how excited I am about this!). Are there any decisions you need from me, or can this be merged?
caveat 1 does sound cumbersome
I agree. The options I currently see are either an opinionated way of setting the environment (e.g. direnv) or introducing a task runner (e.g. just or make), both or which would be quite a change.
Are there any decisions you need from me, or can this be merged?
I rebased the branch and would stick with the initial (current) version:
- locally,
cargo doc --no-depsworks from the workspace root, at the expense of the not-so-nice workarounds forbookandargmin-mathin the CI. - building the
argmin-testfunctionscrate by docs.rs works too.
I rebased the branch and would stick with the initial (current) version:
* locally, `cargo doc --no-deps` works from the workspace root, at the expense of the not-so-nice workarounds for `book` and `argmin-math` in the CI. * building the `argmin-testfunctions` crate by docs.rs works too.
Thanks, this sounds very good to me! Thanks for rebasing. Unfortunately the CI was still broken. Could you do another rebase, please? It should hopefully be fixed now :)
@stefan-k Thanks for the feedback and the main branch fix. I rebased again and all CI jobs pass now on this branch too.