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

Strip out $$ as well #1426

Merged
merged 15 commits into from
Oct 28, 2020
Merged

Strip out $$ as well #1426

merged 15 commits into from
Oct 28, 2020

Conversation

blegat
Copy link
Contributor

@blegat blegat commented Oct 2, 2020

Rebases #1283 and strips $$ as well

Edit by @mortenpi: close #1283, fix #1278

@mortenpi mortenpi added this to the 0.26.0 milestone Oct 3, 2020
@mortenpi
Copy link
Member

mortenpi commented Oct 3, 2020

Thanks for adopting that PR 😄 A few things to think about before we merge:

  1. Will we ever be in a situation where we'll have these delimiters nested? E.g.. \[$$ ... $$\]? I feel that this will always be a bug in the generator of LaTeX.

    So, with that in mind, I am wondering if we should just if ... else match for the patterns and return as soon one matches, instead of passing the output to the next regex?

  2. What are your thoughts on also stripping the $ ... $ and \( ... \)?

  3. As was mentioned in Fix text/latex output in HTMLWriter #1283 (comment), it's not 100% clear what the correct thing to do is if there are no detectable delimiters. We could fall back to Utilities.mdparse, but that means that we require the show methods to print delimiters.. i.e. for example if show prints just x^2 then that won't be rendered as an equation. But this might be fine, since the MIME is text/latex, which is not specific to TeX's math mode.

  4. It would be good to have examples of all the delimiters we handle in the example/test file as well.

@blegat
Copy link
Contributor Author

blegat commented Oct 13, 2020

I agree with 1. and 4, these are addressed in the new version of the PR

About 2. and 3., I agree with

But this might be fine, since the MIME is text/latex, which is not specific to TeX's math mode.

The new version of the PR is non-breaking for cases where it is not wrapped by brackets or $$ so this case may be left for later in case there is anything to be done.

src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
test/examples/src/man/tutorial.md Outdated Show resolved Hide resolved
test/examples/src/man/tutorial.md Outdated Show resolved Hide resolved
@mortenpi mortenpi added the Format: HTML Related to the default HTML output label Oct 21, 2020
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.

Implementation LGTM, just the test examples need fixes. I think we'll merge this after 0.25.3 is tagged.

test/examples/src/man/tutorial.md Outdated Show resolved Hide resolved
test/examples/src/man/tutorial.md Outdated Show resolved Hide resolved
test/examples/src/man/tutorial.md Outdated Show resolved Hide resolved
mortenpi and others added 11 commits October 22, 2020 13:06
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@mortenpi mortenpi merged commit d06fa07 into JuliaDocs:master Oct 28, 2020
@mortenpi
Copy link
Member

Thanks again for adopting and iterating on this @blegat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Format: HTML Related to the default HTML output Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"text/latex" output not rendered
2 participants