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

Rendering LaTeX in markdown #1159

Closed
wants to merge 13 commits into from
Closed

Rendering LaTeX in markdown #1159

wants to merge 13 commits into from

Conversation

arguiot
Copy link
Contributor

@arguiot arguiot commented Feb 14, 2020

As discussed in #1156 , LaTeX rendering inside a code block like:

$$
Equation
$$

Or $Equation$ would be an interesting feature to add, as Apple works on SwiftNumerics and Google on Tensorflow for Swift, many math libraries will start to appear.

This PR focuses on front end rendering, @johnfairh suggested to give control over generating the code element - so instead of <code>$equation$</code> it could give, for example, <span class="math">equation</span>. Then the selector is easier and the styling will be easier because there's no <code> element to fight with.

arguiot and others added 8 commits February 14, 2020 11:41
Using JSDeliver & deferred loading, load time isn't impacted (or very little)
Using JSDeliver & deferred loading, load time isn't impacted (or very little)
Using JSDeliver & deferred loading, load time isn't impacted (or very little)
*Total -- 375.98kb -> 302.06kb (19.66%)

/lib/jazzy/themes/fullwidth/assets/img/dash.png -- 1.31kb -> 0.54kb (58.74%)
/lib/jazzy/themes/jony/assets/img/dash.png -- 1.31kb -> 0.54kb (58.74%)
/lib/jazzy/themes/apple/assets/img/dash.png -- 1.31kb -> 0.54kb (58.74%)
/lib/jazzy/themes/fullwidth/assets/img/gh.png -- 1.53kb -> 0.78kb (48.95%)
/lib/jazzy/themes/jony/assets/img/gh.png -- 1.53kb -> 0.78kb (48.95%)
/lib/jazzy/themes/apple/assets/img/gh.png -- 1.53kb -> 0.78kb (48.95%)
/images/realm.png -- 18.82kb -> 12.25kb (34.91%)
/images/screenshot.jpg -- 233.45kb -> 190.30kb (18.48%)
/images/logo.jpg -- 115.20kb -> 95.55kb (17.06%)

Signed-off-by: ImgBotApp <ImgBotHelp@gmail.com>
[ImgBot] Optimize images
@arguiot
Copy link
Contributor Author

arguiot commented Feb 14, 2020

Also, I'm not sure, but maybe we also should consider passing displayMode: true to KaTeX, since this will center the math on the page on its own line.

@arguiot
Copy link
Contributor Author

arguiot commented Feb 14, 2020

The build is failing because it's comparing the output to a prebuilt version, so if you guys could update that so it succeed

const eq = content.slice(2, -2)
katex.render(eq, block)
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regexps: g flag is not required and will confuse with its lastIndex behaviour. Look at the s flag for the \n|.pattern? Missing ^$ anchors in the bigBlockRegex pattern?

Needs to not match in <pre><code> elements -- that is supposed to be verbatim.

Should test the bigBlockRegex first otherwise small will match (the accidental g flag sometimes saves it!)

Might be better to write a single regexp and use a capture group to pull out the equation to render: what's here gets confused by trailing newlines.

If katex fails then it would be good to fall back to what the user wrote - when I tried I just got blank content.

Style: looks to me as though this file uses semicolons, 2-space indent, and jQuery?

Copy link
Collaborator

@johnfairh johnfairh left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this! Added some javascript nits.

Jazzy docs are supposed to be offline which is how come they ship jQuery and so on.
KaTeX is heavy compared to what we have already, even if we don't ship any of the fonts, so I think to deploy this we'd have to make it opt-in and only copy the js/css/fonts into the docs if the flag is set.

Do you see what I mean about the <code> style? The gray background and h-padding are inherited from there:
Screenshot 2020-02-15 at 11 06 26

I'll try a ruby patch in a bit to generate a span instead -- or a div for the $$ version, that way it'll naturally be a block and easy to style. I agree the $$ should be block & centered like mathoverflow.

Leave out the imagebot stuff - may be worthwhile but should be a different PR, just confusing this one.

To add some tests / make CI happier you need to:

  1. fork jazzy_integration_specs (in the specs/integration_specs) directory
  2. add your tests/demos to the misc_jazzy_features project
  3. do bundle exec rake rebuild_integration_fixtures to update things, check they look right
  4. commit those changes & set up a PR in the jazzy_integration_specs repo
  5. git add spec/integration_specs to this PR and commit it -- this links the new version of the submodule to your PR.

@johnfairh
Copy link
Collaborator

Ok that came out easily enough -- https://gist.github.com/johnfairh/a82dc7cfc927491f31a3d213f8a4a52b
Feel free to tweak the styles etc. / critique my regexes :-)

@arguiot
Copy link
Contributor Author

arguiot commented Mar 1, 2020

@johnfairh the regex looks good to me, I will test with my project and tell you if it works

@arguiot
Copy link
Contributor Author

arguiot commented Mar 1, 2020

@johnfairh I'm having trouble running bundle exec rake rebuild_integration_fixtures.

So, I runned bundle exec rake swift_spec (and added a random function in MiscJazzyFeatures), but your ruby function codespan doesn't seem to have an effect on the final build.

I fixed a few things on the project:

  • revert the ImgBot commit
  • Copied a local version of KaTeX
  • Fixed jazzy.js so it's compatible with your ruby function

Gemfile.lock Outdated
@@ -191,4 +191,4 @@ DEPENDENCIES
webmock

BUNDLED WITH
1.17.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I might broke the build with that!

@johnfairh
Copy link
Collaborator

Hi @arguiot, the Ruby code is removing the $, and CSS class names need a dot in selectors. So the js should be something like:

// KaTeX rendering
document.querySelectorAll('.math').forEach((block) => {
  const content = block.innerHTML
  katex.render(content, block)
});

...which works for me on your fork, html:

<p>How about <span class='math m-inline'>\Gamma(n) = (n-1)!\quad\forall n\in\mathbb N</span> ?</p>

Results:
Screenshot 2020-03-02 at 10 50 36

Yes, undo the Gemfile.lock change.

bundle exec rake swift_spec is a different command: it does not update the test results, it produces output in the jazzy/tmp directory -- I guess your changed files will be in there?

What is the trouble you are having with bundle exec rake rebuild_integration_fixtures ?

If you can add the tests, fix the styling (center/displaymode, anything else?), tweak error handling if possible, then I'll look after the "add a flag to avoid bloating every jazzy site with katex" part.

@arguiot
Copy link
Contributor Author

arguiot commented Mar 8, 2020

@johnfairh Alright, I fixed the problem with bundle exec rake rebuild_integration_fixtures (it was a problem with my local version of ruby in fact) and submitted a new PR https://github.com/arguiot/jazzy-integration-specs . I also fixed the styling.

@johnfairh
Copy link
Collaborator

@arguiot -- just to let you know I'm not ignoring this, slowly working through the pieces.

@arguiot
Copy link
Contributor Author

arguiot commented Mar 15, 2020

No problem

@johnfairh
Copy link
Collaborator

Continued in #1164.

@johnfairh johnfairh closed this Mar 17, 2020
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