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

[main] typeset: add promise-based API #347

Merged
merged 3 commits into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ function ConfigureMathJax() {
AddError("SVG - Unknown character: U+"+message[1].toString(16).toUpperCase()+
" in "+(message[2].fonts||["unknown"]).join(","),!undefinedChar);
});
MathJax.Hub.Register.MessageHook("CommonHTML Jax - unknown char",function (message) {
AddError("CHTML - Unknown character: U+"+message[1].toString(16).toUpperCase()+
" in "+(message[2].fonts||["unknown"]).join(","),!undefinedChar);
});
MathJax.Hub.Register.MessageHook("MathML Jax - unknown node type",function (message) {
AddError("MathML - Unknown node type: "+message[1]);
});
Expand Down Expand Up @@ -825,7 +829,9 @@ function RestartMathJax() {
// %%% cache results?
// %%% check types and values of parameters
//
exports.typeset = function (data,callback) {

// callback API for compatibility with MathJax
var cbTypeset = function (data, callback) {
if (!callback || typeof(callback) !== "function") {
if (displayErrors) console.error("Missing callback");
return;
Expand All @@ -841,6 +847,17 @@ exports.typeset = function (data,callback) {
if (serverState == STATE.READY) StartQueue();
}

// main API, callback and promise compatible
exports.typeset = function (data, callback) {
if (callback) cbTypeset(data, callback);
else return new Promise(function (resolve, reject) {
cbTypeset(data, function (output, input) {
if (output.errors) reject(output.errors);
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to be worried about the fact that output.errors an include warnings (like missing characters)? There is a configuration option for turning this off, so it may not be necessary to worry about that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have said no since undefinedCharError is false by default (and if you switch it on, you likely want to reject the promise).

However, I have noticed that switching it to true has no effect for the promise-based API, e.g.,

var mjAPI = require("./lib/main.js");
mjAPI.config({undefinedCharError: true});

mjAPI.typeset({
    math: "呵呵",
    format: "TeX",
    htmlNode: true,
    html: true,
    svg: true,
    equationNumbers: "AMS"
}, function (data) {
    console.log(data.errors); // has errors
});


mjAPI.typeset({math:'呵呵 ', html:true})
    .then((output, input) => console.log(output))
    .catch(errors => console.log("Errors:",errors)); //does not have errors

I don't understand what's going on here. (Hard errors work fine by the way, e.g., in the test file.)

Copy link
Member

Choose a reason for hiding this comment

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

The problem turns out to be that the CHTML undefined character signals are not being trapped — only the SVG ones. So your first typeset() call shows them because it has svg: true, but the second doesn't, because it is only for html.

You can fix this by adding

      MathJax.Hub.Register.MessageHook("CommonHTML Jax - unknown char",function (message) {
        AddError("CHTML - Unknown character: U+"+message[1].toString(16).toUpperCase()+
                    " in "+(message[2].fonts||["unknown"]).join(","),!undefinedChar);
      });

just after the corresponding SVG version at around line 198.

Copy link
Member

Choose a reason for hiding this comment

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

I would have said no since undefinedCharError is false by default

I thought it was true, but I see that you are right, so I agree, no need to do anything about it. But at least we did find out that the CHTML undefined character signals aren't currently being trapped. :-)

else resolve(output, input);
});
});
};

//
// Manually start MathJax (this is done automatically
// when the first typeset() call is made)
Expand Down
22 changes: 22 additions & 0 deletions test/base-typeset-promise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
var tape = require('tape');
var mjAPI = require("../lib/main.js");

tape('basic test: check typeset promise API', function (t) {
t.plan(2);

var tex = '';
mjAPI.start();

// promise resolved
mjAPI.typeset({
math: tex,
format: "TeX",
mml: true
}).then((result) => t.ok(result.mml, 'Typset promise resolved on success'));

mjAPI.typeset({
math: tex,
format: "MathML",
mml: true
}).catch((error) => t.ok(error, 'Typeset promise rejected on error'));
});
11 changes: 11 additions & 0 deletions test/base-warnings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
var tape = require('tape');
var mjAPI = require("../lib/main.js");
mjAPI.config({undefinedCharError: true});

tape('basic test: check warnings', function (t) {
t.plan(2);
mjAPI.typeset({math:'\u5475', html:true})
.catch(errors => t.ok(errors, 'CommonHTML output reports error'));
mjAPI.typeset({math:'\u5475', svg:true})
.catch(errors => t.ok(errors, 'SVG output reports error'));
});