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

Swap KaTeX from local to global #372

Merged
merged 2 commits into from
May 17, 2024
Merged

Swap KaTeX from local to global #372

merged 2 commits into from
May 17, 2024

Conversation

felixhekhorn
Copy link
Contributor

As ekore is at its kore a math package there are a lot of math formulas in the docs and we are using KaTeX to render them.

On master we were using katexit for a dynamic injection of the KaTeX (HTML) header files. In practice they will appear everywhere which is annoying.

So the idea here is to change to a global injection as is proposed here (with an updated KaTeX version).

Question is: will this work on docs.rs? Does someone of you know by chance @alecandido or @cschwan ? otherwise I guess the only way is to try and see what is happening.

This would require us to actually start publishing eko stuff to docs.rs - how is that done in practice? with our current layout there are 2 packages: ekore and ekors (which is the glue between python eko (i.e. numba) and Rust ekore) (+ #260 would introduce a third package: dekoder).

(Fixes to rustify.sh are taken from #369 , which eventually should be rebased onto this PR)

@felixhekhorn felixhekhorn added documentation Improvements or additions to documentation rust Rust extension related labels May 16, 2024
@alecandido
Copy link
Member

@felixhekhorn I was thinking to do the exact same in a Qibo project (I could link the issue, but it's not yet public).

On docs.rs it will work as much as katexit, since it is just running rustdoc/cargo doc under the hood, and serving the result (without almost any limitation server side... you could even abuse it, https://docs.rs/pwnies/0.0.13/pwnies/).

This would require us to actually start publishing eko stuff to docs.rs - how is that done in practice?

Automatically: you publish your projects on crates.io, with cargo publish (possibly in the CI, but definitely not required), and it will trigger the docs build from the repo, eventually publishing on docs.rs.

@alecandido
Copy link
Member

ekors could be just eko in Rust, if there is anything beyond the Python bindings. ekors is only required for Python (to disambiguate with the obvious eko package).
I obtained that name some time ago writing to the previous owner:
https://crates.io/crates/eko

So the idea here is to change to a global injection as is proposed here (with an updated KaTeX version).

The trade-off between global injection and local one is that:

  • the local one (by katexit) is uselessly repeated for the many objects that could be on the same page
  • the global one will be present even on pages that do not contain latex formulas

However, dropping katexit will definitely be beneficial, since you will remove an unneeded dependency, and all the attribute macros that you had to spread in the source (keeping source cleaner is more relevant than keeping the generated assets lighter - though it's a trade-off, and both is obviously best).

@felixhekhorn
Copy link
Contributor Author

  • the global one will be present even on pages that do not contain latex formulas

as said above: I expect math to be almost everywhere

re publish: mmm, it seems not super trivial to me, so let's split the discussion from here into a separate issue ...

However, I take it that you, @alecandido, generally agree with the idea here and so I will merge this PR

@felixhekhorn felixhekhorn merged commit 3c86e0e into master May 17, 2024
8 checks passed
@felixhekhorn felixhekhorn deleted the swap-katex branch May 17, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation rust Rust extension related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants