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

fix inline mathjax handling #195

Merged
merged 4 commits into from
Dec 20, 2023
Merged

Conversation

chrisbresten
Copy link
Contributor

@chrisbresten chrisbresten commented Oct 21, 2023

previously it was converting mathjax unsupported inline math delimiters $ to block math delimiters $$ which are not equivalent. here it instead converts inline math delim $ to the mathjax supported inline math delim \( \) which have the expected equivalent behavior

Copy link
Collaborator

@pbodnar pbodnar left a comment

Choose a reason for hiding this comment

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

Hi @chrisbresten, thank you for the PR. I've got some suggestions and questions.

Also, there is this unit test https://github.com/miyuchina/mistletoe/blob/v1.2.1/test/test_contrib/test_mathjax.py which needs amendments, so that it doesn't newly fail. (BTW its testing markdown is pretty "artificial", I think it would deserve quite a rewrite - at minimum to use some "real" math expressions like 2^1 in it.)

mistletoe/contrib/mathjax.py Outdated Show resolved Hide resolved
mistletoe/contrib/mathjax.py Outdated Show resolved Hide resolved
mistletoe/contrib/mathjax.py Outdated Show resolved Hide resolved
@pbodnar pbodnar added this to the 1.3.0 milestone Oct 25, 2023
@pbodnar pbodnar added the bug label Oct 25, 2023
@pbodnar
Copy link
Collaborator

pbodnar commented Dec 9, 2023

@chrisbresten, would you like to finish the small "cosmetic modifications" as suggested above, or can I take it over? :)

@chrisbresten
Copy link
Contributor Author

@chrisbresten, would you like to finish the small "cosmetic modifications" as suggested above, or can I take it over? :)

hi, i am sorry for the slow response. I am not that well versed in the style conventions and cleanliness standards for production in this context so i had kind of deferred to you implicitly. I should have stated this.

@chrisbresten
Copy link
Contributor Author

i checked my local copy and found changes to unit tests to reflect the new behavior properly. I defer to you @pbodnar for any changes that help maintain standards and style consistent with the codebase at large

@coveralls
Copy link

coveralls commented Dec 20, 2023

Coverage Status

coverage: 94.162% (+0.09%) from 94.073%
when pulling 62d8104 on chrisbresten:inline_mathjax
into 70e8e8d on miyuchina:master.

@pbodnar pbodnar merged commit 8bc913d into miyuchina:master Dec 20, 2023
9 checks passed
@pbodnar
Copy link
Collaborator

pbodnar commented Dec 20, 2023

@chrisbresten, thanks once again. I did the final amendments and hopefully didn't break anything. ;)

@pbodnar
Copy link
Collaborator

pbodnar commented Dec 20, 2023

BTW I have found this cool "cheatsheet" for the MathJax syntax: https://math.meta.stackexchange.com/questions/5020/mathjax-basic-tutorial-and-quick-reference/.

@chrisbresten
Copy link
Contributor Author

BTW I have found this cool "cheatsheet" for the MathJax syntax: https://math.meta.stackexchange.com/questions/5020/mathjax-basic-tutorial-and-quick-reference/.

thankyou. I write a lot of latex and there can be a lot of variation in what kind of syntax people choose to use out of habbit. there are a lot of options for some functionality due to improvements manifesting as new commands in order to leave the old ones for backward compatibility. also a lot of packages people use so ubiquitously that they forget the corresponding functionality is not builtin. mathjax seems to do a good job at keeping a pulse on this and providing a math environment that is compliant with this implicit standard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants