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

Wrap LaTeXString with text{} #381

Merged
merged 2 commits into from
Sep 14, 2020
Merged

Wrap LaTeXString with text{} #381

merged 2 commits into from
Sep 14, 2020

Conversation

zmoon
Copy link
Contributor

@zmoon zmoon commented Sep 7, 2020

Wrap LaTeXString with text{} in order to preserve mixed text/math in the rendered repr using MathJax.

#359

@zmoon
Copy link
Contributor Author

zmoon commented Sep 7, 2020

Is the texed = repr(mime, x) line still necessary? What else is needed?

@fonsp
Copy link
Owner

fonsp commented Sep 8, 2020

How did the MathJax issue go?

@zmoon
Copy link
Contributor Author

zmoon commented Sep 8, 2020

Good, they said the fix should be in the next patch release.

@fonsp
Copy link
Owner

fonsp commented Sep 14, 2020

@zmoon MathJax released an update, can you check if it works with their latest version? Can you update the version that Pluto is using?

@zmoon
Copy link
Contributor Author

zmoon commented Sep 14, 2020

It seems like there are still issues after the initial fix: mathjax/MathJax#2524 (comment)

Pluto specifies mathjax@3 so it should use the latest npm version without changing anything.

<script src="common/SetupMathJax.js"></script>
<script type="text/javascript" id="MathJax-script" src="https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-svg-full.js" async></script>
</head>

@fonsp
Copy link
Owner

fonsp commented Sep 14, 2020

Oh that should be a full version instead, can you set it to 3.1.2?

The new bug looks fine to ignore

@zmoon
Copy link
Contributor Author

zmoon commented Sep 14, 2020

Seems to work.

image

@fonsp
Copy link
Owner

fonsp commented Sep 14, 2020

Thanks!

@fonsp fonsp marked this pull request as ready for review September 14, 2020 23:58
@fonsp fonsp merged commit 1aa22b2 into fonsp:master Sep 14, 2020
@zmoon zmoon deleted the latexstrings-repr branch September 15, 2020 00:20
@stillyslalom
Copy link

MathJax isn't meant for text-first rendering, and treating it otherwise seems to be causing more problems than it fixes. Better to use native Markdown constructs to handle text/inline math:
image

@zmoon
Copy link
Contributor Author

zmoon commented Nov 11, 2020

The goal here was just to make LaTeXStrings show nicely in Pluto. They are supposed to represent text-first LaTeX (consistent with the MIME type text/latex, afaict). Writing in the Pluto notebook it is certainly more convenient to use Markdown, though $...$ is more common than ``...`` I would think.

For the short term it seems like it might be better for LaTeXStrings display to be special-cased...?

@stillyslalom
Copy link

But the display for MIME"text/latex" is done by MathJax, which isn't meant for text-first rendering - due to the lack of line-wrapping, it can't display more than a few words of text, and certainly not a paragraph.

image

@fonsp
Copy link
Owner

fonsp commented Nov 11, 2020

LaTeXStrings is such a mess oof

@zmoon
Copy link
Contributor Author

zmoon commented Nov 12, 2020

But the display for MIME"text/latex" is done by MathJax, which isn't meant for text-first rendering - due to the lack of line-wrapping, it can't display more than a few words of text, and certainly not a paragraph.

Yeah, it's not optimal, but I think it works fine for the main use case of LaTeXStrings - constructing labels and annotations for plots. But that shouldn't come at the expense of other packages' displays.

Maybe in the future the text/latex MIME type could be treated in a more complex way, leaving normal text as normal text and giving the LaTeX environments and math text to MathJax. This seems to be what IPython display does in Jupyter. Seems like using Markdown.parse instead of Markdown.LaTeX might do this using the standard Julia Markdown machinery like @stillyslalom suggests.

@fonsp
Copy link
Owner

fonsp commented Nov 12, 2020

That doesn't work for \begin{equation}

@zmoon
Copy link
Contributor Author

zmoon commented Nov 12, 2020

Doesn't seem to be a problem
image

$$...$$ is not recognized the same way that it is in md"..." though.
Edit: actually I guess just need to escape both of them even though using L"...".

Same result

image

image

@fonsp
Copy link
Owner

fonsp commented Nov 13, 2020

I am very confused, can you write it without a L_str inside Markdown.parse? Or was that the point?

@fonsp
Copy link
Owner

fonsp commented Nov 13, 2020

It would be really really helpful if you write a PR for this an create a notebook showing off all the things that we want to support. If not then maybe it's best to drop support for LaTeXStrings again

@zmoon
Copy link
Contributor Author

zmoon commented Nov 13, 2020

I am very confused, can you write it without a L_str inside Markdown.parse? Or was that the point?

I used L_str partially for convenience and partially to check it would work for LaTeXString type. But I can confirm it works the same for normal string too as long as things are escaped properly.

It would be really really helpful if you write a PR for this an create a notebook showing off all the things that we want to support.

I will see what I can do!

@stillyslalom
Copy link

I'd recommend using @raw_str for this sort of thing:

image

@fonsp
Copy link
Owner

fonsp commented Nov 13, 2020

@zmoon Great! Just the testing notebook would already be a big help

@zmoon
Copy link
Contributor Author

zmoon commented Nov 14, 2020

@fonsp I made a notebook testing some options here

What I discovered is that although my simple example with Markdown.parse above worked without proper math delimiting, in general that is not the case (e.g., if the equations have multiple _ or * in them). That is (JuliaLang/julia#37334), either $...$ starting on same line or the Julia Markdown math delimiters should be used. Not everything that should be rendered by MathJax comes in with the proper delimiters to mark it so (e.g., \begin{equation}..., since it is valid LaTeX without math delimiters), so some parsing/processing would be necessary (e.g., with mixed text and \begin{equation}... to add math delimiters to the equation part).

The issue is that while text/latex MIME type may not be, Markdown.LaTeX and MathJax are math-first. But most things that use text/latex in the notebook context seem to be math-only or at least math-first. So I guess some options are

  1. revert to the previous usage of Markdown.LaTeX without \text and either
    a. special-case LaTeXStrings
    b. or not
  2. implement parsing text/latex and adding math delimiters where necessary to support mixed text/math (like LaTeX does), showing it in the notebook as mixed normal text and MathJax similar to Jupyter (in Jupyter notebook at least, seems to be done in the js part)

Seems like 1. is the way to go for now?

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.

None yet

3 participants