Skip to content
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
52 changes: 32 additions & 20 deletions MJxPrep.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,6 @@ if (window.MJxPrep) {
eqn = wrapVariables(eqn)
eqn = wrapFuncCalls(eqn)

// wrapFuncCalls correctly transforms 'conj(z)', but
// wrapFuncCalls turns 'conj(z' into '{:conj:}(' and
// wrapVariables turns 'conj' into '{:conj:}', This is problematic...
// conj is defined below as a <mover/> symbol. edX version of asciimath
// has trouble with <mover/> symbols wrapped directly in non-breaking group.
// This hack avoids lonely, non-breaking, <mover/> symbols.
// See https://github.com/mitodl/mitx-grading-library/issues/163 for more.
eqn = eqn.replace(/\{:conj:\}/g, "{:text(co)text(nj):}");

// Return the preprocessed equation
return eqn;
}
Expand All @@ -109,7 +100,7 @@ if (window.MJxPrep) {
function updateMathJax() {
if (MathJax.InputJax.AsciiMath) {
// Grab the AsciiMath object
AM = MathJax.InputJax.AsciiMath.AM;
var AM = MathJax.InputJax.AsciiMath.AM;

// All of these new symbols are based on those appearing in the AsciiMath definitions
// See https://github.com/asciimath/asciimathml/blob/master/ASCIIMathML.js
Expand Down Expand Up @@ -291,18 +282,40 @@ if (window.MJxPrep) {
// Check for the AsciiMath object every 200ms
var checkExist = setInterval(updateMathJax, 200);

// set of AsciiMath symbols classified as mover (math-overscript) tags
// (This should be a Set object, but ie11 doesn't fully support Sets)
OVERSCRIPT_SYMBOLS = {
bar: true,
conj: true,
ddot: true,
dot: true,
hat: true,
obrace: true,
overarc: true,
overbrace: true,
overline: true,
overparen: true,
overset: true,
stackrel: true,
tilde: true,
vec: true
}

// callback function for String.prototype.replace
function wrapIfNotOverscript(match, substr) {
return OVERSCRIPT_SYMBOLS[substr]
? substr
: '{:' + substr + ':}'
}

/**
* Detect variables, then wrap them in invisible brackets
* Without wrapping symbols, expressions like x'/x render poorly
*
* @param {string} eqn
* @return {string}
*/
function wrapVariables(eqn) {
// Wrap invisibrackets around variables and functions
var wrap_group = function(match, substr) {
return '{:' + substr + ':}';
};

// Need a regex to match any possible variable name
// This regex matches all valid expressions in the python parser
// If invalid expressions are given, this is less predictable, but the wrapping shouldn't hurt anything
Expand Down Expand Up @@ -333,14 +346,14 @@ if (window.MJxPrep) {
(?![(}]) # Negative lookahead: '(' (function) or '}' (tensor indices)
*/
// This site is really useful for debugging: https://www.regextester.com/
eqn = eqn.replace(var_expr, wrap_group);
eqn = eqn.replace(var_expr, wrapIfNotOverscript);

return eqn
}

/**
* Detect function calls, then wrap them in invisible brackets
*
* Without wrapping function calls, expressions like p(x)/2 render poorly
* @param {string} eqn
* @return {string}
*/
Expand Down Expand Up @@ -371,14 +384,13 @@ if (window.MJxPrep) {
var funcEnd = bracketCloses === null ? bracketOpens : bracketCloses + 1

var front = eqn.slice(0, funcStart)
var middle = '{:' + eqn.slice(funcStart, funcEnd) + ':}'
eqn = front + middle + eqn.slice(funcEnd)
var middle = eqn.slice(funcStart, funcEnd)
eqn = front + wrapIfNotOverscript(null, middle) + eqn.slice(funcEnd)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is unnecessary, as nothing in the set contains parentheses, and this is only called for completed functions, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wrapFuncCalls wraps incomplete function calls:

wrapFuncCalls('cat(') // returns '{:cat:}(
wrapVariables('cat(') // returns 'cat('

Now, I'm not sure that wrapping incomplete function calls is actually necessary... I think when we encountered the issues with x'/2 and p(x)/2, we adopted "wrap everything" as a philosophy, so I wrapped incomplete function calls, too. (Overscript issues has made me question that philosophy...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, the point was more that the old code is equivalent to the new code, unless I'm missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that the test I added for wrapFuncCalls describes how we want it to behave for overscript symbols:

https://github.com/mitodl/mitx-grading-library/pull/170/files#diff-1d7052fc002008ab0813d9d4de53d3b7R191

The middle two cases for that test require this alteration to wrapFuncCalls.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see now. Very good! Thanks for the clarifications. For posterity, I'll add that overline() converts to {:overline():}, which is the desired behavior, and works in AsciiMath.

}

return eqn
}


/**
* get index at which the brace at braceIdx in expr is closed, or null if it
* does not close.
Expand Down
29 changes: 29 additions & 0 deletions mitxgraders-js/MJxPrep.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ describe('wrapVariables', () => {
}
} )

it('does not wrap isolated overscript symbols', () => {

const exprs = [
['a+overline+b', '{:a:}+overline+{:b:}']
]

for (const pair of exprs) {
const [testCase, expected] = pair
expect(wrapVariables(testCase)).toBe(expected)
}

} )

} )

describe('wrapFuncCalls', () => {
Expand All @@ -175,6 +188,22 @@ describe('wrapFuncCalls', () => {
}
} )

it('only wraps overscript symbols if function call is complete', () => {

const exprs = [
['a+overline+b', 'a+overline+b'],
['a+overline(+b', 'a+overline(+b'],
['a+overline(x+b', 'a+overline(x+b'],
['a+overline(x)+b', 'a+{:overline(x):}+b']
]

for (const pair of exprs) {
const [testCase, expected] = pair
expect(wrapFuncCalls(testCase)).toBe(expected)
}

} )

} )


Expand Down