Skip to content

Clear user defined TeX macros from global state #348

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Clear user defined TeX macros from global state #348

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f4f2296
Clear user defined TeX macros from global state
rianmcguire Aug 3, 2017
cb238ef
Allow macros to be persisted using state
rianmcguire Aug 3, 2017
d2f8491
Track user-defined macros by overriding setDef
rianmcguire Aug 8, 2017
8cacb49
Track user-defined environments by overriding setEnv
rianmcguire Aug 8, 2017
5b549fe
Add missing hasOwnProperty checks
rianmcguire Aug 8, 2017
b10feb6
Add comments
rianmcguire Aug 8, 2017
540b5e7
Pin Mathjax Version to 2.7.6
Sep 5, 2019
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
21 changes: 21 additions & 0 deletions lib/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,19 @@ function GetState(state) {
MML.SUPER.ID = ID = 0;
MathJax.OutputJax.CommonHTML.ID = 0;
}

// Clear any existing user defined macros, then load macros from state
Object.keys(TEX.Definitions.macros).forEach(function(macroName){
Copy link
Member

Choose a reason for hiding this comment

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

Since you use TeX.Definitions.macros a number of times, here, perhaps

var MACROS = TeX.Definitions.macros;

would be in order.

(Also, there should be a space between the ) and the following {.)

if (TEX.Definitions.macros[macroName].isUser) {
delete TEX.Definitions.macros[macroName];
}
});
if (state && state.macros) {
for (var macroName in state.macros) {
Copy link
Member

Choose a reason for hiding this comment

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

You should use if (state.macros.hasOwnProperty(macroName) {...} inside this loop, so that you will be sure to only use the names that are actually in the object. For example, if someone augments the Object.prototype object (and we have encountered that before), then you will loop through this additions as well.

TEX.Definitions.macros[macroName] = state.macros[macroName];
TEX.Definitions.macros[macroName].isUser = true;
Copy link
Member

Choose a reason for hiding this comment

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

Should not need to set this, as it is already set in the objects that are pointed to in state.macros.

Copy link
Author

Choose a reason for hiding this comment

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

I figured if someone were to serialize the state to JSON, the isUser property would be lost because the macro is an array.

If that's not something you want to support, I'm happy to remove it. It's not important for our use case at the moment.

}
}
}

//
Expand All @@ -765,6 +778,7 @@ function ReturnResult(result) {
if (state) {
var AMS = MathJax.Extension["TeX/AMSmath"];
var GLYPH = MathJax.OutputJax.SVG.BBOX.GLYPH;
var TEX = MathJax.InputJax.TeX;
state.AMS.startNumber = AMS.startNumber;
state.AMS.labels = AMS.labels;
state.AMS.IDs = AMS.IDs;
Expand All @@ -773,6 +787,13 @@ function ReturnResult(result) {
state.defs = GLYPH.defs;
state.n = GLYPH.n;
state.ID = ID;
state.macros = {};
for (var macroName in TEX.Definitions.macros) {
Copy link
Member

Choose a reason for hiding this comment

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

Again, if (TeX.Definitions.macros.hasOwnProperty(maroName)) {...} should be used inside this loop.

var macro = TEX.Definitions.macros[macroName];
if (macro.isUser) {
state.macros[macroName] = macro;
}
}
}
serverState = STATE.READY;
callback(result, originalData);
Expand Down
60 changes: 60 additions & 0 deletions test/macro-state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
var tape = require('tape');
var mjAPI = require("../lib/main.js");

tape('macros should be cleared from global state', function(t) {
t.plan(2);
mjAPI.start();

mjAPI.typeset({
math: "\\def\\mymacro{2\\pi} \\mymacro",
format: "TeX",
mml: true
}, function(data) {
t.false(data.errors, 'Defined and used macro');
});

mjAPI.typeset({
math: "\\mymacro",
format: "TeX",
mml: true
}, function(data) {
t.true(data.errors, '\\mymacro should no longer be defined');
});
});

tape('macros can be persisted using state option', function(t) {
t.plan(4);
mjAPI.start();

var state = {};

mjAPI.typeset({
math: "\\def\\mymacro{2\\pi} \\mymacro",
format: "TeX",
mml: true,
state: state
}, function(data) {
t.false(data.errors, 'Defined and used macro');
t.equal(Object.keys(state.macros).length, 1, 'Only stores the user defined macro in state');

// Ensure state contains only serializable data
state = JSON.parse(JSON.stringify(state));

mjAPI.typeset({
math: "\\mymacro",
format: "TeX",
mml: true,
state: state
}, function(data) {
t.false(data.errors, '\\mymacro was not persisted in state');
});

mjAPI.typeset({
math: "\\mymacro",
format: "TeX",
mml: true,
}, function(data) {
t.true(data.errors, '\\mymacro should no longer be defined if state is not provided');
});
});
});