From 485c50987943f20fc72c02b28d70e8ceea6422e3 Mon Sep 17 00:00:00 2001 From: Erik Demaine Date: Mon, 21 May 2018 22:56:34 -0400 Subject: [PATCH] Cleanup MathML , , (#1338) * Avoid unnecessary wrapping buildMathML gains two helpers: * `makeRow` helper wraps an array of nodes in ``, unless the array has length 1, in which case no wrapping is necessary. * `buildExpressionRow` for common case of `makeRow(buildExpression(...))` * Combine adjacent s in all cases No more need for `makeTextRow` helper or anything fancy in text MathML handler. * Concatenate s and decimal point into single Fix #203 * Fix snapshots --- src/buildMathML.js | 99 ++++++++++++-------------- src/functions/delimsizing.js | 4 +- src/functions/genfrac.js | 4 +- src/functions/href.js | 4 +- src/functions/mathchoice.js | 8 +-- src/functions/text.js | 2 +- test/__snapshots__/mathml-spec.js.snap | 92 +++++++++++++----------- test/mathml-spec.js | 4 ++ 8 files changed, 104 insertions(+), 113 deletions(-) diff --git a/src/buildMathML.js b/src/buildMathML.js index e6e84f40..b2f1ffd4 100644 --- a/src/buildMathML.js +++ b/src/buildMathML.js @@ -28,32 +28,15 @@ export const makeText = function(text, mode) { return new mathMLTree.TextNode(text); }; -export const makeTextRow = function(body, options) { - // Convert each element of the body into MathML, and combine consecutive - // outputs into a single tag. In this way, we don't - // nest non-text items (e.g., $nested-math$) within an . - const inner = []; - let currentText = null; - for (let i = 0; i < body.length; i++) { - const group = buildGroup(body[i], options); - if (group.type === 'mtext' && currentText !== null) { - Array.prototype.push.apply(currentText.children, group.children); - } else { - inner.push(group); - if (group.type === 'mtext') { - currentText = group; - } else { - currentText = null; - } - } - } - - // If there is a single tag in the end (presumably ), - // just return it. Otherwise, wrap them in an . - if (inner.length === 1) { - return inner[0]; +/** + * Wrap the given array of nodes in an node if needed, i.e., + * unless the array has length 1. Always returns a single node. + */ +export const makeRow = function(body) { + if (body.length === 1) { + return body[0]; } else { - return new mathMLTree.MathNode("mrow", inner); + return new mathMLTree.MathNode("mrow", body); } }; @@ -97,11 +80,7 @@ export const getVariant = function(group, options) { export const groupTypes = {}; groupTypes.ordgroup = function(group, options) { - const inner = buildExpression(group.value, options); - - const node = new mathMLTree.MathNode("mrow", inner); - - return node; + return buildExpressionRow(group.value, options); }; groupTypes.supsub = function(group, options) { @@ -119,18 +98,15 @@ groupTypes.supsub = function(group, options) { } } - const removeUnnecessaryRow = true; const children = [ - buildGroup(group.value.base, options, removeUnnecessaryRow)]; + buildGroup(group.value.base, options)]; if (group.value.sub) { - children.push( - buildGroup(group.value.sub, options, removeUnnecessaryRow)); + children.push(buildGroup(group.value.sub, options)); } if (group.value.sup) { - children.push( - buildGroup(group.value.sup, options, removeUnnecessaryRow)); + children.push(buildGroup(group.value.sup, options)); } let nodeType; @@ -167,11 +143,11 @@ groupTypes.supsub = function(group, options) { groupTypes.tag = function(group, options) { const table = new mathMLTree.MathNode("mtable", [ new mathMLTree.MathNode("mlabeledtr", [ - new mathMLTree.MathNode("mtd", - buildExpression(group.value.tag, options)), new mathMLTree.MathNode("mtd", [ - new mathMLTree.MathNode("mrow", - buildExpression(group.value.body, options)), + buildExpressionRow(group.value.tag, options), + ]), + new mathMLTree.MathNode("mtd", [ + buildExpressionRow(group.value.body, options), ]), ]), ]); @@ -181,14 +157,30 @@ groupTypes.tag = function(group, options) { /** * Takes a list of nodes, builds them, and returns a list of the generated - * MathML nodes. A little simpler than the HTML version because we don't do any - * previous-node handling. + * MathML nodes. Also combine consecutive outputs into a single + * tag. */ export const buildExpression = function(expression, options) { const groups = []; + let lastGroup; for (let i = 0; i < expression.length; i++) { - const group = expression[i]; - groups.push(buildGroup(group, options)); + const group = buildGroup(expression[i], options); + // Concatenate adjacent s + if (group.type === 'mtext' && lastGroup && lastGroup.type === 'mtext') { + lastGroup.children.push(...group.children); + // Concatenate adjacent s + } else if (group.type === 'mn' && + lastGroup && lastGroup.type === 'mn') { + lastGroup.children.push(...group.children); + // Concatenate ... followed by . + } else if (group.type === 'mi' && group.children.length === 1 && + group.children[0].text === '.' && + lastGroup && lastGroup.type === 'mn') { + lastGroup.children.push(...group.children); + } else { + groups.push(group); + lastGroup = group; + } } // TODO(kevinb): combine \\not with mrels and mords @@ -196,13 +188,19 @@ export const buildExpression = function(expression, options) { return groups; }; +/** + * Equivalent to buildExpression, but wraps the elements in an + * if there's more than one. Returns a single node instead of an array. + */ +export const buildExpressionRow = function(expression, options) { + return makeRow(buildExpression(expression, options)); +}; + /** * Takes a group from the parser and calls the appropriate groupTypes function * on it to produce a MathML node. */ -export const buildGroup = function( - group, options, removeUnnecessaryRow = false, -) { +export const buildGroup = function(group, options) { if (!group) { return new mathMLTree.MathNode("mrow"); } @@ -210,11 +208,6 @@ export const buildGroup = function( if (groupTypes[group.type]) { // Call the groupTypes function const result = groupTypes[group.type](group, options); - if (removeUnnecessaryRow) { - if (result.type === "mrow" && result.children.length === 1) { - return result.children[0]; - } - } return result; } else { throw new ParseError( @@ -234,7 +227,7 @@ export default function buildMathML(tree, texExpression, options) { const expression = buildExpression(tree, options); // Wrap up the expression in an mrow so it is presented in the semantics - // tag correctly. + // tag correctly, unless it's a single or . let wrapper; if (expression.length === 1 && utils.contains(["mrow", "mtable"], expression[0].type)) { diff --git a/src/functions/delimsizing.js b/src/functions/delimsizing.js index 7a2e70d5..7b7f3388 100644 --- a/src/functions/delimsizing.js +++ b/src/functions/delimsizing.js @@ -261,9 +261,7 @@ defineFunction({ inner.push(rightNode); } - const outerNode = new mathMLTree.MathNode("mrow", inner); - - return outerNode; + return mml.makeRow(inner); }, }); diff --git a/src/functions/genfrac.js b/src/functions/genfrac.js index ab0a38a8..e35fec90 100644 --- a/src/functions/genfrac.js +++ b/src/functions/genfrac.js @@ -243,9 +243,7 @@ defineFunction({ withDelims.push(rightOp); } - const outerNode = new mathMLTree.MathNode("mrow", withDelims); - - return outerNode; + return mml.makeRow(withDelims); } return node; diff --git a/src/functions/href.js b/src/functions/href.js index d96049fd..bcb0a601 100644 --- a/src/functions/href.js +++ b/src/functions/href.js @@ -1,7 +1,6 @@ // @flow import defineFunction, {ordargument} from "../defineFunction"; import buildCommon from "../buildCommon"; -import mathMLTree from "../mathMLTree"; import {assertNodeType} from "../ParseNode"; import * as html from "../buildHTML"; @@ -35,8 +34,7 @@ defineFunction({ return new buildCommon.makeAnchor(href, [], elements, options); }, mathmlBuilder: (group, options) => { - const inner = mml.buildExpression(group.value.body, options); - const math = new mathMLTree.MathNode("mrow", inner); + const math = mml.buildExpressionRow(group.value.body, options); math.setAttribute("href", group.value.href); return math; }, diff --git a/src/functions/mathchoice.js b/src/functions/mathchoice.js index 561c2b25..05509289 100644 --- a/src/functions/mathchoice.js +++ b/src/functions/mathchoice.js @@ -1,7 +1,6 @@ // @flow import defineFunction, {ordargument} from "../defineFunction"; import buildCommon from "../buildCommon"; -import mathMLTree from "../mathMLTree"; import Style from "../Style"; import * as html from "../buildHTML"; import * as mml from "../buildMathML"; @@ -47,11 +46,6 @@ defineFunction({ }, mathmlBuilder: (group, options) => { const body = chooseMathStyle(group, options); - const elements = mml.buildExpression( - body, - options, - false - ); - return new mathMLTree.MathNode("mrow", elements); + return mml.buildExpressionRow(body, options); }, }); diff --git a/src/functions/text.js b/src/functions/text.js index 2b6e088c..9888b150 100644 --- a/src/functions/text.js +++ b/src/functions/text.js @@ -62,6 +62,6 @@ defineFunction({ return buildCommon.makeSpan(["mord", "text"], inner, newOptions); }, mathmlBuilder(group, options) { - return mml.makeTextRow(group.value.body, options); + return mml.buildExpressionRow(group.value.body, options); }, }); diff --git a/test/__snapshots__/mathml-spec.js.snap b/test/__snapshots__/mathml-spec.js.snap index 96c9258c..138404b0 100644 --- a/test/__snapshots__/mathml-spec.js.snap +++ b/test/__snapshots__/mathml-spec.js.snap @@ -57,6 +57,35 @@ exports[`A MathML builder accents turn into in MathML 1`] `; +exports[`A MathML builder should concatenate digits into single 1`] = ` + + + + + + sin + + + ⁡ + + + α + + + = + + + 0.34 + + + + \\sin{\\alpha}=0.34 + + + + +`; + exports[`A MathML builder should generate nodes for \\phantom 1`] = ` @@ -87,11 +116,9 @@ exports[`A MathML builder should generate the right types of nodes 1`] = ` ⁡ - - - x - - + + x + + @@ -339,11 +366,9 @@ exports[`A MathML builder should render mathchoice as if there was nothing 3`] = x - - - T - - + + T + @@ -367,11 +392,9 @@ exports[`A MathML builder should render mathchoice as if there was nothing 4`] = y - - - T - - + + T + @@ -387,8 +410,8 @@ exports[`A MathML builder should set href attribute for href appropriately 1`] = - - + + α @@ -406,11 +429,9 @@ exports[`A MathML builder should use for colorbox 1`] = ` - - - b - - + + b + @@ -427,11 +448,9 @@ exports[`A MathML builder should use for raisebox 1`] = ` - - - b - - + + b + @@ -507,22 +526,9 @@ exports[`A MathML builder tags use 1`] = ` - - - ( - - - - h - - - i - - - - ) - - + + (hi) + diff --git a/test/mathml-spec.js b/test/mathml-spec.js index 43afbe86..57f5c436 100644 --- a/test/mathml-spec.js +++ b/test/mathml-spec.js @@ -35,6 +35,10 @@ describe("A MathML builder", function() { expect(getMathML("\\sin{x}+1\\;\\text{a}")).toMatchSnapshot(); }); + it('should concatenate digits into single ', () => { + expect(getMathML("\\sin{\\alpha}=0.34")).toMatchSnapshot(); + }); + it('should make prime operators into nodes', () => { expect(getMathML("f'")).toMatchSnapshot(); });