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

Deduplicate math code and include KaTeX CSS automatically when needed #2717

Merged
merged 13 commits into from Apr 13, 2017

Conversation

@Kwpolska
Copy link
Member

Kwpolska commented Apr 12, 2017

This is #2715.

Kwpolska added 3 commits Apr 12, 2017
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@ralsina
Copy link
Member

ralsina commented Apr 12, 2017

LGTM!

Kwpolska added 6 commits Apr 12, 2017
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska requested a review from ralsina Apr 12, 2017
@Kwpolska
Copy link
Member Author

Kwpolska commented Apr 12, 2017

I’d like to request another review, since I changed quite a few more things.

@@ -0,0 +1,58 @@
{% macro math_scripts() %}

This comment has been minimized.

Copy link
@felixfontein

felixfontein Apr 12, 2017

Contributor

How about adding two macros (one for KaTeX, one for MathJax) which provide the version number for each of them?

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Apr 13, 2017

Author Member

Not possible since we’re using checksums.

This comment has been minimized.

Copy link
@felixfontein

felixfontein Apr 13, 2017

Contributor

Ah, I overlooked that.

@@ -972,13 +974,10 @@ PRETTY_URLS = ${PRETTY_URLS}

# Want to use KaTeX instead of MathJax? While KaTeX is less featureful,
# it's faster and the output looks better.
# If you set USE_KATEX to True, you also need to add an extra CSS file like
# this: (make sure to match the version Nikola uses -- check it in HTML output)
# EXTRA_HEAD_DATA = """<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/katex.min.css" integrity="sha384-wITovz90syo1dJWVh32uuETPVEtGigN07tkttEqPv+uR2SE/mbQcG7ATL28aI9H0" crossorigin="anonymous">"""

This comment has been minimized.

Copy link
@felixfontein

felixfontein Apr 12, 2017

Contributor

Maybe we should add some code in Nikola.__init__ which checks for "KaTeX/0.7.1/katex.min.css" in self.config["EXTRA_HEAD_DATA"] and informs the user about these changes, in case they missed the changelog?

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Apr 13, 2017

Author Member

Sure, will do. (It won’t ever be 0.7.1 in a user config, because that was only a stopgap change)

Kwpolska added 4 commits Apr 13, 2017
h/t @felixfontein

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska merged commit cc3bafd into master Apr 13, 2017
3 of 5 checks passed
3 of 5 checks passed
continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@Kwpolska Kwpolska deleted the math_helper branch Apr 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.