-
Notifications
You must be signed in to change notification settings - Fork 100
Preliminary {math }
#811
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
Preliminary {math }
#811
Conversation
I would tend to say that KaTeX should not be vendored, especially since it means we need a webserver to serve the content. Regarding inline/block ... what about using the surrounding context ? If it's in a block context, it's a block, otherwise, it's inlined ? The question of static generation remains. We should indeed make an option for that. Is there no other alternative math-to-mathml converter ? What about HeVea ? |
Why would a web server be needed to serve the content more than what's currently done with highlight.pack.js or odoc.css if KaTeX was vendored? It's just having the files accessible with the right path in the end, just like any other page For the inline/block, that sounds reasonable! Avoids syntax overload, and it's pretty clear. I like the idea of depending on KaTeX, given that it is widely used, and maintained by people I tend trust. HeVea seems risky, I trust the maintainers just as much as the KaTeX maintainers, but don't know how much time they can spend on it. Also, it is a quite big project, and I think it's only an executable, built with a Makefile, no dune etc.. so making it a library would be very painful, and I don't think I'm not sure it can handle tex fragments anyway. The optimal case would be to have a reliable JS interpreter written in OCaml that could simply run KaTeX without needing node installed, but I don't think that exists. |
It's quite heavy though, e.g. if you want to write the equivalent of |
As I said, it adds a lot to the parser for something that is kind of already there. Implementing {m } and {%math %} would be mostly duplicating the logic of raw_markup in the parser. |
Note I'm note suggesting |
Yes, see the PR I drafted, the block part is actually quite trivial. I actually prefer |
I don't see a problem with that, it makes sense to me. What I write here specifically targets "maths", which may or may not be supported by a given backend. It's easier and more regular if we don't introduce a new syntax for that. |
It's the same syntax for a different node in the AST, but I'm happy changing it! |
Hello, would it be possible to get a review on the syntax upgrade ? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a small problem with Raw_markup
, backends are free to ignore markup they don't recognize. I expect the math
markup to always be rendered, at least as an unstyled code span.
There's potentially many backends, for example the future documentation website or the new markdown generator.
Is someone writing alternative markup for every backends (eg. the same text but formatted differently) ? I suspect this is not how it is most used in practice and this is not compatible with allowing third party backends to exist.
Should we change the meaning of Raw_markup
to be rendered at least as a code span on every backends ?
I'm fine with fetching katex from a CDN (I'm also fine with either distributing it with Odoc or not providing it at all), however there's a huge problem:
I don't want Katex to be loaded if it is not used. It should be loaded dynamically if it is needed (script and style).
Just a thought: We could add options to html-generate
to load arbitrary scripts and styles. This way, the katex part of this PR can be implemented separately.
For now Dune doesn't allow passing arbitrary options but this should be fixed in Dune rather than worked around here.
This works well with the change to Raw_markup
I proposed above. The HTML backend could generate a class name from the raw markup's target
to be able to recognize them from user scripts.
Answering the review on the syntax update here at the same time. First of all answering these two comments:
and
I do think that this is why we shouldn't be using Because odoc-parser is mostly only used by odoc itself, I don't think it's an issue to change the constraint on odoc-parser's version in this PR.
That is a very good point actually! But I believe this could be decided at compile time? Because pages are generated separately, and the header is added to each page, we could simply detect the presence of |
Indeed, changing Most of the matches are for images, which are always html-only. Some examples:
A math syntax seems needed. There's dozens of instances of math notations written in code spans or other weird ways.
There's literally one instance of the raw markup syntax being used to have alternative rendering in different backends. Of course, it is for math notation:
Is Opium using its own language ? There's weird things like:
and
I was surprised to not find many
I think it's worth reconsidering this feature. Having new syntaxes for images and math would remove almost all the current uses for raw markup.
How do they differ? There's no
It is used by mdx and ocamlformat (for which an AST change is breaking). Though, if we actually want to change the parser, we can fix this by vendoring the parser in ocamlformat for example.
Compiler time is even better. The runtime solution is more suitable to be implemented outside of Odoc, with my raw_markup proposal. |
In what I'd want to do, writing Ah, I just checked mdx and ocamlformat opam files, and it is a bit annoying that they depend on ocamlformat >= 1.0.0 and not I do agree that raw_markup would be worth reconsidering, but maybe simply deprecated? Maybe there's some very old code out there that uses it. |
We can decide (and I'm rather in favour) that math is sufficiently important to deserve some mainline syntax and dedicated handling. cc @jonludlam :) Compile-time detection for call to katex is a good idea. |
I like having a specific math-specific syntax rather than just hijacking the backend-specific syntax.
I don't think we should change the meaning |
+1, but please no CDN for katex or
If the |
We discussed this yesterday. Everyone agree that we should not use Since we are making it a "proper" construct, Let's drop the |
The issue is that the alternative is to vendor ~70 files for KaTeX or MathJax. In the case of odig, it would probably mean that these 70 files are copy-pasted for every library, which is probably wasted space One work-around could be:
The annoying thing is that, detecting if KaTeX is needed is easy for individual pages, so we can decide if we need the header at compile time, but it's difficult for dune to decide if the support files are needed or not... But well, it's 70 files, but they will only be made the first time you build the doc [EDIT: I originally suggested default behaviour should be to use a CDN, but if the things are vendored anyway, the only thing we need to go around is the massive copy-pasting for odig] |
Why would that be the case ? There's a single assets directory for the whole doc set. This should simply be added to |
Ah, I didn't know it was the case, then it's perfect. Sorry for over-thinking this haha Good - then I'll add that to the PR, and I'll modify the syntax PR to use |
Gah, sorry I dropped the ball on that one again :( Thank you for continuing it :) |
No worries, your contributions live on in the new PR - if you felt like reviewing it that'd be useful though :-) |
This is just to make progress on the discussion for #123.
This is a very simple implementation, and there are several points to discuss:
defer
should probably be added to the css and js loading, to reduce the amount of time it takes to load the page.math
andmathi
. From the katex point of view, the only difference is adisplayMode: true
parameter for the render function