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.

Sign up for GitHub

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

Jump to bottom

MathJax3 support #1367

Merged
merged 12 commits into from
Aug 6, 2020
Merged

MathJax3 support #1367

merged 12 commits into from
Aug 6, 2020

Conversation

simeonschaub
Copy link
Contributor

The options for MathJax3 are currently ignored, but the default rendering already works.
Should close #1362

@simeonschaub simeonschaub marked this pull request as draft July 15, 2020 00:24
@mortenpi mortenpi added Format: HTML Related to the default HTML output Type: Enhancement labels Jul 16, 2020
@simeonschaub
Copy link
Contributor Author

Everything should work now. Do you think that's the right approach?

@simeonschaub simeonschaub marked this pull request as ready for review July 26, 2020 10:41
@simeonschaub
Copy link
Contributor Author

I don't really know how the tests here work, so please tell me what still needs to be added in that department. I was thinking that maybe we should already rename MathJax to MathJax2 and make MathJax just an alias, so we can make v3 the default eventually. Or do you think it would be better to do this at a later time?

@simeonschaub simeonschaub changed the title WIP: MathJax3 support MathJax3 support Jul 26, 2020
@mortenpi
Copy link
Member

mortenpi commented Jul 26, 2020 •
edited
Loading

I don't really know how the tests here work, so please tell me what still needs to be added in that department.

The front end is not really being systematically tested. What would be nice, though, is to make sure that one of the builds in test/examples/make.jl would actually use MathJax3. This should make sure that this code path does not throw an error at least.

Edit: I missed that you already have it. Just need to make sure that we have a build for both the old MathJax2 and the new build. This may mean adding another build with another combination of configuration options.

I was thinking that maybe we should already rename MathJax to MathJax2 and make MathJax just an alias, so we can make v3 the default eventually. Or do you think it would be better to do this at a later time?

No, this would be good to do, if you don't mind. MathJax should stay as an alias, yea, for now. Preferably one that prints a deprecation warning. I think it could just be a function prints the deprecation and then calls MathJax2(args...; kwargs...).

mortenpi
mortenpi reviewed Jul 27, 2020
View reviewed changes
test/examples/src/man/tutorial.md
@@ -367,6 +367,7 @@ mathengine = Documenter.MathJax(Dict(:TeX => Dict(

```math
\bra{x}\ket{y}
\pdv[n]{f}{x}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something that should only work in MathJax3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to double check that the physics package was actually working.

src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
test/examples/make.jl Outdated Show resolved Hide resolved
@simeonschaub
Copy link
Contributor Author

I have no clue what's going on with the test failures...

@simeonschaub simeonschaub reopened this Jul 28, 2020
@simeonschaub
Copy link
Contributor Author

simeonschaub commented Jul 28, 2020 •
edited
Loading

🎉 Well, that took a while too figure out. Still don't really understand what was going on here, but it works now.

@simeonschaub
Copy link
Contributor Author

Did you have time to take another look at this?

mortenpi
mortenpi reviewed Aug 1, 2020
View reviewed changes
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I took the liberty of organizing the tests slightly. Just one doc question, but otherwise it's good to go as far as I can tell 🙂

src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
@mortenpi mortenpi modified the milestones: 0.25.1, 0.25.2 Aug 1, 2020
mortenpi and others added 3 commits August 1, 2020 12:44
@simeonschaub
Copy link
Contributor Author

Sorry for the delay! I went into a bit more detail about which options are set by default in the docstring. This should now be good to go from my side.

@simeonschaub simeonschaub mentioned this pull request Aug 5, 2020
Merged
@mortenpi
Copy link
Member

mortenpi commented Aug 6, 2020

LGTM, thanks a lot @simeonschaub! I will aim to tag a new release with this in the next few days.

@mortenpi mortenpi merged commit b66e4ca into JuliaDocs:master Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mortenpi mortenpi mortenpi left review comments

Assignees
No one assigned
Labels
Format: HTML Related to the default HTML output Type: Enhancement
Projects
None yet
Milestone
0.25.2
Development

Successfully merging this pull request may close these issues.

Add MathJax3 Writer
2 participants
@simeonschaub @mortenpi