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

Improve MathJax compatibility with myst-parser #577

Merged
merged 2 commits into from
Jul 17, 2021

Conversation

chrisjsewell
Copy link
Contributor

@chrisjsewell chrisjsewell commented Jun 17, 2021

Heya, this would meet me halfway with executablebooks/MyST-Parser#395, so both packages will share the same processClass regex (and myst-parser won't complain about overriding it).
Plus anyway it is slightly better for compatibility with any advanced use cases, so people can still use the MathJax standard classes tex2jax_process (mathjax2) and mathjax_process (mathjax3).

@pep8speaks
Copy link

pep8speaks commented Jun 17, 2021

Hello @chrisjsewell! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-06-17 22:11:42 UTC

@chrisjsewell
Copy link
Contributor Author

line too long (83 > 79 characters)

get with the times, use black formatting and 88 character lines 😜

@mgeier
Copy link
Member

mgeier commented Jun 18, 2021

Thanks for this PR!

I have nothing against this change, but just for my understanding, who/what generates those classes tex2jax_process and mathjax_process?

line too long (83 > 79 characters)

That's just the opinion of the bot, feel free to ignore it.

use black formatting

I won't. I don't like it.

and 88 character lines

Only in exceptional cases.

@chrisjsewell
Copy link
Contributor Author

cheers

but just for my understanding, who/what generates those classes tex2jax_process and mathjax_process

well nothing in sphinx/docutils core, but these are the default classes that MathJax itself sets to mark elements whose contents should be processed (in v2 and v3 respectively) and so, one would imagine, that this should really be the canonical way that any third-party extensions and/or advanced use cases should (or already do) mark elements for processing.

@mgeier
Copy link
Member

mgeier commented Jun 20, 2021

Thanks, I wasn't aware that those options have default values. See http://docs.mathjax.org/en/v3.1-latest/options/document.html?highlight=mathjax_process#option-descriptions.

With the argument of better compatibility, shouldn't the value ignoreHtmlClass: 'mathjax_ignore' (and 'tex2jax_ignore' for MathJax 2) be kept as well?

@mgeier mgeier merged commit f9852f8 into spatialaudio:master Jul 17, 2021
@mgeier
Copy link
Member

mgeier commented Jul 17, 2021

I've added the missing "ignore" defaults in #585.

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