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

Add math engine based on MathJax-node that outputs to MathML #240

Closed
wants to merge 6 commits into from
Closed

Add math engine based on MathJax-node that outputs to MathML #240

wants to merge 6 commits into from

Conversation

@tmthrgd
Copy link
Contributor

@tmthrgd tmthrgd commented Apr 5, 2015

This adds a new math engine based on MathJax-node. MathJax-node runs the MathJax conversion in node.js. mathjaxnode.rb converts the TeX code to MathML in a node.js subprocess using the MathJax-node bundled tex2mml command.

@gettalong gettalong self-assigned this Apr 5, 2015
cmd << "--inline" unless type == :block
cmd << "--semantics" if converter.options[:math_engine_opts][:semantics] == true
cmd << "--notexhints" if converter.options[:math_engine_opts][:texhints] == false
result = IO.popen(cmd << el.value).read.strip
Copy link
Owner

@gettalong gettalong Apr 5, 2015

Better not use a pipe when a simple result = #{cmd << el.value}.strip would suffice since some systems might not support pipes.

Copy link
Contributor Author

@tmthrgd tmthrgd Apr 5, 2015

It's not quite that easy. The raw value of the TeX is passed in the command line. If you use #{cmd << el.value}, or similar, it becomes trivial to craft input that will execute arbitrary programs. Using IO.popen with an array as the first argument causes ruby bypasses the shell: "If cmd is an Array of String, then it will be used as the subprocess’s argv bypassing a shell.". Doing it this way prevents those exploits.

Copy link
Owner

@gettalong gettalong Apr 5, 2015

Okay, thanks for the explanation. I have been using the systemu library in such cases which should work on any system. Just that I know: Would IO.popen work on Windows?

Copy link
Contributor Author

@tmthrgd tmthrgd Apr 5, 2015

It certainly should. It appears that systemu itself calls IO.popen to execute commands on all systems, with the exception of JRuby which is using java.lang.Runtime.runtime.exec.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 5, 2015

Thanks for this math engine!

As for the name: I haven't done anything with node.js yet but I gather that node.js does have a Markdown library, so the target audience would be Ruby developers. Would it be better to name this engine something like 'MathJax-MathML'?

As for testing: Do you know what to put into the .travis.yml file to get the needed binaries and libraries? Some more explanation on the documentation page would also help people who do not know about node.js. I.e. explaining that an external program called node.js has to be installed as well as npm (if it is not packaged with node.js) and then the MathJax-Node library. (This would also help me set up my machine correctly for testing the engine ;-))

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Apr 5, 2015

I named it to be consistent with the underlying library but what your saying does make sense. I'll look into the .travis.yml and installing node.js bit at some point. Node.js just seems like ruby or python to me, I just sort of expect it to be there in my terminal. I'll get onto that though and clarify things.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 5, 2015

Thanks!

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Apr 5, 2015

All the errors attributed to 'Problem using tidy' look something like this:

line 1 column 1 - Warning: missing <!DOCTYPE> declaration
line 1 column 1 - Warning: inserting implicit <body>
line 1 column 12 - Error: <math> is not recognized!
line 1 column 12 - Warning: discarding unexpected <math>
line 2 column 3 - Error: <mi> is not recognized!
line 2 column 3 - Warning: discarding unexpected <mi>
line 2 column 8 - Warning: discarding unexpected </mi>
line 3 column 3 - Error: <mo> is not recognized!
line 3 column 3 - Warning: discarding unexpected <mo>
line 3 column 25 - Warning: discarding unexpected </mo>
line 4 column 3 - Error: <mi> is not recognized!
line 4 column 3 - Warning: discarding unexpected <mi>
line 4 column 8 - Warning: discarding unexpected </mi>
line 5 column 3 - Error: <mo> is not recognized!
line 5 column 3 - Warning: discarding unexpected <mo>
line 5 column 25 - Warning: discarding unexpected </mo>
line 6 column 3 - Error: <mo> is not recognized!

When tidy is removed the tests pass on my machine but fail when tidy is installed.

Edit: I have solved all of the tidy issues; I hadn't noticed the exclusions.

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Apr 5, 2015

Unfortunately IO.popen has a different, incompatible, signature on ruby 1.8.7 which is causing that particular build to fail. All others are succeeding. The two solutions I can come up with are either: removing support for ruby 1.8.7 (extended maintenance did end July 31, 2014), or selectively disable Mathjax-Node on ruby < 1.9.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 5, 2015

You can just remove support for Ruby 1.8.7 from the engine since the engine isn't bound by backwards compatibility yet.

Please also state this in its documentation!

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Apr 5, 2015

Hurrah! All tests are passing. It's your call on the name though.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 5, 2015

Thanks for all your work! Regarding the name: I will let the information sink in some time and then decide.

This will be in the next release.

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Apr 5, 2015

👍 You're welcome.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 26, 2015

@tmthrgd I'm just integrating your code for the next release. After some problems with nodejs/npm that I needed to fix the tests work fine. However, they take ages to run. Is node.js that slow? Or has this to do with MathJax-node?

Anyway, the additional tests take a whooping 20 seconds to execute whereas the whole test suite needed 35 seconds before... That's not optimal. Do you know of a way to speed up the MathJax-node tests?

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 27, 2015

I have merged your commits into one and pushed to master, ie. your changes are now live.

Regarding the slow tests: I will modify the test suite so that slow tests are not executed depending on an environment variable.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 17, 2016

@tmthrgd New commits fail in Travis due to an error with tex2mml: Unknown argument: f(x) = a{x^3} + b{x^2} + cx + d.
See for example https://travis-ci.org/gettalong/kramdown/builds/109940440.

When I test locally everything works fine - do you know why this happens?

@tmthrgd
Copy link
Contributor Author

@tmthrgd tmthrgd commented Feb 18, 2016

@gettalong I was able to recreate the error locally after running sudo npm --global install MathJax-node. While I have no idea what was causing the problem, it is fixed by instead using the mathjax-node.

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

2 participants