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

Pass one shared macros object into every call to katex renderer #148006

Merged
merged 3 commits into from
May 2, 2022

Conversation

AlbertHilb
Copy link
Contributor

This PR fixes #125425

In order to make macros defined by the author persistent between KaTeX elements, we need to pass one shared macros object into every call to the renderer. KaTeX will insert macros into that object and since it continues to exist between calls, macros will persist. See KaTeX docs.

immagine

@ghost
Copy link

ghost commented Apr 24, 2022

CLA assistant check
All CLA requirements met.

@Walker-Xin
Copy link

Walker-Xin commented Apr 27, 2022

The issue of katex macros not persisting across math expressions also occurs in Jupyter notebook environment of VSC. Would this PR resolve the macro issue for Jupyter notebooks?

@AlbertHilb
Copy link
Contributor Author

Yes, this PR also solves the issue for notebooks.

screen

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 30, 2022

What if you delete a previously declared macro? With this approach won't it (incorrectly) still be defined?

@AlbertHilb
Copy link
Contributor Author

Yes, Indeed. That behavior is certainly incorrect for the markdown file preview. The new version of the patch solves the issue resetting the macros object on the start of each new re-rendering.

screen

This approach does not work for notebooks since each markdown cell is a standalone markdown document.

However, in this case, I'm not sure the actual behavior is an issue and needs to be corrected. In some sense, it seems to match the way the notebook works. Code cells act similarly. If you set a variable in a cell, execute the cell and then remove it, the variable is still accessible in the rest of the notebook.

Maybe we can add a command to reset the markdown engine just like the command to reload the kernel. What do you think?

@mjbvz mjbvz merged commit 7e6b007 into microsoft:main May 2, 2022
@mjbvz
Copy link
Collaborator

mjbvz commented May 2, 2022

Thanks. The new behavior sounds reasonable enough for notebooks too so we can merge as-is and wait for feedback

@mjbvz mjbvz added this to the May 2022 milestone May 2, 2022
@AlbertHilb AlbertHilb deleted the PersistentMacros branch May 2, 2022 16:50
@chandresh
Copy link

@AlbertHilb I tested this and the issue (#125425) that I reported related to markdown /necommand in Jypitor notebooks is working fine with your fix. Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using VSCode insiders: Jupyter Notebook Markdown Cell does not support \newcommand in Katex.
4 participants