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

Add katex support in documentation. #427

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

mmaker
Copy link
Member

@mmaker mmaker commented Jun 10, 2022

This commit adds latex support within rustdoc for arkworks algebra.
In order to add latex code in textmode, use delimiters $ and $.
In order to add latex code in displaymode, use delimiters $$ and $$.
To compile the documentation, use cargo rustdoc.

This is achieved via the option --html-in-header set via

  • the environment variable RUSTDOCFLAGS, or
  • the variable build.rustdocflags of .cargo/config.toml
    I opted for the latter making it a default compilation flag to use with
    rustdoc.

IMPORTANT: for some reason (independent from us) cargo rustdoc fails
on rust nightly and this is due to a failure concerning cfg-if and
rand-chacha. I did not investigate the reason why this happened.

@mmaker
Copy link
Member Author

mmaker commented Jun 10, 2022

Great, right now doctests (cargo test) fail because the documentation path is incorrect:
instead of doc/katex-header.html it expects ../doc/katex-header.html.
However, cargo rustdoc expects doc/katex-header.html instead of ../doc/katex-header.html.

We have two options:

  • we YOLO it, and let the user set the option --html-in-header or the env variable RUSTDOCFLAGS manually.
  • we create a makefile inside each package with the following content:
doc:
	cargo rustdoc -- --html-in-header doc/katex-header.html

The latter is what's being used in curve25519-dalek, but would pollute the build environment :(
This affects only local compilation and shouldn't concern upload to docs.rs, which is already dealt with.

@Pratyush
Copy link
Member

@mmaker would using the solution in this issue work? rust-lang/cargo#9427

We would have to switch to running doctests on nightly only, but that's fine IMO

@mmaker
Copy link
Member Author

mmaker commented Jun 11, 2022

After some investigation:

rustdocflags = "-Zdoctest-in-workspace --html-in-header doc/katex-header.html"

clearly does not work because it's not a rustdoc option. After some hacking around, the following option is what we want:

[unstable]
doctest-in-workspace = true

Now .cargo/config.html looks like the following:

# Add support for katex, and work around rust-lang issue #8097.
# <https://github.com/rust-lang/cargo/issues/8097>
[target.'cfg(nightly)']
rustdocflags = "--html-in-header doc/katex-header.html"

# Without this line, one among `cargo test --doc` and `cargo rustdoc`
# will fail due to the relative path in the `--html-in-header` option for rustdoc.
[unstable]
doctest-in-workspace = true

Note the target.cfg(nightly): without it, on rust stable we'd have cargo test --doc fail because of the relative path issue...
However, now we have nother problem: target.<cfg> sections only define rustflags and not rustdocflags... therefore we're back to square one.

This commit adds latex support within rustdoc for arkworks algebra.
In order to add latex code in textmode, use delimiters `$` and `$`.
In order to add latex code in displaymode, use delimiters `$$` and `$$`.
To compile the documentation, use `make doc`.

Building documentation with support for latex requires an additional
option --html-in-header to be set via RUSTDOCFLAGS or build.rustdocflags
in .cargo/config.toml.
Unfortunately, putting a relative path there causes `cargo test`
and `cargo doc` to fail. See rust-lang issue #8097.
Working around this bug with the option `-Zdoctest-in-workspace`
requires enabling rust toolchain nightly. Unfortunately, there
is no such option as `target.'cfg(nightly)'.rustdocflags` to work with.
Work around all these issues with a single makefile.
@mmaker mmaker force-pushed the feature/doc-katex branch from b3182e7 to 1287dd5 Compare June 11, 2022 18:12
@mmaker
Copy link
Member Author

mmaker commented Jun 11, 2022

Force-pushing this pull request with a makefile-based workaround, similarly to what's done in curve25519-dalek...

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