Repository: Khan/KaTeX
Pull request #1541 (2018-08-02 03:40:49)

Introduce "atom" parse node to coalesce various symbol nodes.
Towards #1492.


codecov[bot]: # [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=h1) Report
> Merging [#1541](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=desc) into [master](https://codecov.io/gh/Khan/KaTeX/commit/30be53efe6f174a8e25bc866ffe5b91fce81f47b?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `90.62%`.


[![Impacted file tree graph](https://codecov.io/gh/Khan/KaTeX/pull/1541/graphs/tree.svg?width=650&height=150&src=pr&token=vfDnj2P77o)](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1541      +/-   ##
==========================================
- Coverage   84.11%   84.11%   -0.01%     
==========================================
  Files          79       79              
  Lines        4558     4563       +5     
  Branches      803      807       +4     
==========================================
+ Hits         3834     3838       +4     
- Misses        629      630       +1     
  Partials       95       95
```


| [Impacted Files](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/utils.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL3V0aWxzLmpz) | `66.66% <ø> (ø)` | :arrow_up: |
| [src/symbols.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL3N5bWJvbHMuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/functions/mclass.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL2Z1bmN0aW9ucy9tY2xhc3MuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/Parser.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL1BhcnNlci5qcw==) | `93.94% <100%> (+0.08%)` | :arrow_up: |
| [src/functions/symbolsOp.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL2Z1bmN0aW9ucy9zeW1ib2xzT3AuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/functions/genfrac.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL2Z1bmN0aW9ucy9nZW5mcmFjLmpz) | `87.77% <50%> (ø)` | :arrow_up: |
| [src/parseNode.js](https://codecov.io/gh/Khan/KaTeX/pull/1541/diff?src=pr&el=tree#diff-c3JjL3BhcnNlTm9kZS5qcw==) | `84.21% <83.33%> (-1.51%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=footer). Last update [30be53e...4378ca0](https://codecov.io/gh/Khan/KaTeX/pull/1541?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/Parser.js b/src/Parser.js
index c258b67c..9a92c533 100644
--- a/src/Parser.js
+++ b/src/Parser.js
@@ -3,7 +3,7 @@
 import functions from "./functions";
 import environments from "./environments";
 import MacroExpander from "./MacroExpander";
-import symbols, {extraLatin} from "./symbols";
+import symbols, {ATOMS, extraLatin} from "./symbols";
 import {validUnit} from "./units";
 import {supportedCodepoint} from "./unicodeScripts";
 import unicodeAccents from "./unicodeAccents";
@@ -16,6 +16,7 @@ import Settings from "./Settings";
 import SourceLocation from "./SourceLocation";
 import {Token} from "./Token";
 import type {AnyParseNode, SymbolParseNode} from "./parseNode";
+import type {Atom, Group} from "./symbols";
 import type {Mode, ArgType, BreakToken} from "./types";
 import type {FunctionContext, FunctionSpec} from "./defineFunction";
 import type {EnvSpec} from "./defineEnvironment";
@@ -1022,14 +1023,28 @@ export default class Parser {
                     `Latin-1/Unicode text character "${text[0]}" used in ` +
                     `math mode`, nucleus);
             }
-            // TODO(#1492): Remove this override once this becomes an "atom" type.
-            // $FlowFixMe
-            const s: SymbolParseNode = {
-                type: symbols[this.mode][text].group,
-                mode: this.mode,
-                loc: SourceLocation.range(nucleus),
-                value: text,
-            };
+            const group: Group = symbols[this.mode][text].group;
+            const loc = SourceLocation.range(nucleus);
+            let s: SymbolParseNode;
+            if (ATOMS.hasOwnProperty(group)) {
+                // $FlowFixMe
+                const family: Atom = group;
+                s = {
+                    type: "atom",
+                    mode: this.mode,
+                    family,
+                    loc,
+                    value: text,
+                };
+            } else {
+                // $FlowFixMe
+                s = {
+                    type: group,
+                    mode: this.mode,
+                    loc,
+                    value: text,
+                };
+            }
             symbol = s;
         } else if (text.charCodeAt(0) >= 0x80) { // no symbol for e.g. ^
             if (this.settings.strict) {
diff --git a/src/functions/genfrac.js b/src/functions/genfrac.js
index 5fb9a740..9068bed6 100644
--- a/src/functions/genfrac.js
+++ b/src/functions/genfrac.js
@@ -4,7 +4,7 @@ import buildCommon from "../buildCommon";
 import delimiter from "../delimiter";
 import mathMLTree from "../mathMLTree";
 import Style from "../Style";
-import {assertNodeType, checkNodeType} from "../parseNode";
+import {assertNodeType, assertAtomFamily, checkNodeType} from "../parseNode";
 
 import * as html from "../buildHTML";
 import * as mml from "../buildMathML";
@@ -370,17 +370,17 @@ defineFunction({
         // Look into the parse nodes to get the desired delimiters.
         let leftNode = checkNodeType(args[0], "ordgroup");
         if (leftNode) {
-            leftNode = assertNodeType(leftNode.value[0], "open");
+            leftNode = assertAtomFamily(leftNode.value[0], "open");
         } else {
-            leftNode = assertNodeType(args[0], "open");
+            leftNode = assertAtomFamily(args[0], "open");
         }
         const leftDelim = delimFromValue(leftNode.value);
 
         let rightNode = checkNodeType(args[1], "ordgroup");
         if (rightNode) {
-            rightNode = assertNodeType(rightNode.value[0], "close");
+            rightNode = assertAtomFamily(rightNode.value[0], "close");
         } else {
-            rightNode = assertNodeType(args[1], "close");
+            rightNode = assertAtomFamily(args[1], "close");
         }
         const rightDelim = delimFromValue(rightNode.value);
 
diff --git a/src/functions/mclass.js b/src/functions/mclass.js
index 7ada92d1..f62f861f 100644
--- a/src/functions/mclass.js
+++ b/src/functions/mclass.js
@@ -45,15 +45,14 @@ defineFunction({
     mathmlBuilder,
 });
 
-export const binrelClass = (arg: AnyParseNode) => {
+export const binrelClass = (arg: AnyParseNode): string => {
     // \binrel@ spacing varies with (bin|rel|ord) of the atom in the argument.
     // (by rendering separately and with {}s before and after, and measuring
     // the change in spacing).  We'll do roughly the same by detecting the
     // atom type directly.
-    const atomType = (arg.type === "ordgroup" &&
-        arg.value.length ? arg.value[0].type : arg.type);
-    if (/^(bin|rel)$/.test(atomType)) {
-        return "m" + atomType;
+    const atom = (arg.type === "ordgroup" && arg.value.length ? arg.value[0] : arg);
+    if (atom.type === "atom" && (atom.family === "bin" || atom.family === "rel")) {
+        return "m" + atom.family;
     } else {
         return "mord";
     }
diff --git a/src/functions/symbolsOp.js b/src/functions/symbolsOp.js
index a8244381..79f685ed 100644
--- a/src/functions/symbolsOp.js
+++ b/src/functions/symbolsOp.js
@@ -5,48 +5,26 @@ import mathMLTree from "../mathMLTree";
 
 import * as mml from "../buildMathML";
 
-import type Options from "../Options";
-import type {ParseNode} from "../parseNode";
-import type {Group} from "../symbols";
-
 // Operator ParseNodes created in Parser.js from symbol Groups in src/symbols.js.
 
-// NOTE: `NODETYPE` is constrained by `Group` instead of `NodeType`. This
-// guarantees that `group.value` is a string as required by buildCommon.mathsym.
-function defineOpFunction<NODETYPE: Group>(
-    type: NODETYPE,
-    mathmlNodePostProcessor?: (
-        mathMLTree.MathNode,
-        ParseNode<NODETYPE>,
-        Options) => *,
-) {
-    defineFunctionBuilders({
-        type,
-        htmlBuilder(group: ParseNode<NODETYPE>, options) {
-            const groupValue: string = group.value;
-            return buildCommon.mathsym(
-                groupValue, group.mode, options, ["m" + type]);
-        },
-        mathmlBuilder(group: ParseNode<NODETYPE>, options) {
-            const node = new mathMLTree.MathNode(
-                "mo", [mml.makeText(group.value, group.mode)]);
-            if (mathmlNodePostProcessor) {
-                mathmlNodePostProcessor(node, group, options);
+defineFunctionBuilders({
+    type: "atom",
+    htmlBuilder(group, options) {
+        return buildCommon.mathsym(
+            group.value, group.mode, options, ["m" + group.family]);
+    },
+    mathmlBuilder(group, options) {
+        const node = new mathMLTree.MathNode(
+            "mo", [mml.makeText(group.value, group.mode)]);
+        if (group.family === "bin") {
+            const variant = mml.getVariant(group, options);
+            if (variant === "bold-italic") {
+                node.setAttribute("mathvariant", variant);
             }
-            return node;
-        },
-    });
-}
-
-defineOpFunction("bin", (mathNode, group: ParseNode<"bin">, options) => {
-    const variant = mml.getVariant(group, options);
-    if (variant === "bold-italic") {
-        mathNode.setAttribute("mathvariant", variant);
-    }
+        } else if (group.family === "punct") {
+            node.setAttribute("separator", "true");
+        }
+        return node;
+    },
 });
-defineOpFunction("rel");
-defineOpFunction("open");
-defineOpFunction("close");
-defineOpFunction("inner");
-defineOpFunction("punct", mathNode => mathNode.setAttribute("separator", "true"));
 
diff --git a/src/parseNode.js b/src/parseNode.js
index 26b2bbe4..9b628f60 100644
--- a/src/parseNode.js
+++ b/src/parseNode.js
@@ -1,7 +1,8 @@
 // @flow
-import {GROUPS} from "./symbols";
+import {NON_ATOMS} from "./symbols";
 import type SourceLocation from "./SourceLocation";
 import type {ArrayEnvNodeData} from "./environments/array";
+import type {Atom} from "./symbols";
 import type {Mode, StyleStr} from "./types";
 import type {Token} from "./Token";
 import type {Measurement} from "./units";
@@ -18,15 +19,10 @@ export type LeftRightDelimType = {|
 
 // ParseNode's corresponding to Symbol `Group`s in symbols.js.
 export type SymbolParseNode =
+    ParseNode<"atom"> |
     ParseNode<"accent-token"> |
-    ParseNode<"bin"> |
-    ParseNode<"close"> |
-    ParseNode<"inner"> |
     ParseNode<"mathord"> |
     ParseNode<"op-token"> |
-    ParseNode<"open"> |
-    ParseNode<"punct"> |
-    ParseNode<"rel"> |
     ParseNode<"spacing"> |
     ParseNode<"textord">;
 
@@ -162,26 +158,9 @@ type ParseNodeTypes = {
     // From symbol groups, constructed in Parser.js via `symbols` lookup.
     // (Some of these have "-token" suffix to distinguish them from existing
     // `ParseNode` types.)
-    "accent-token": {|
-        type: "accent-token",
-        mode: Mode,
-        loc?: ?SourceLocation,
-        value: string,
-    |},
-    "bin": {|
-        type: "bin",
-        mode: Mode,
-        loc?: ?SourceLocation,
-        value: string,
-    |},
-    "close": {|
-        type: "close",
-        mode: Mode,
-        loc?: ?SourceLocation,
-        value: string,
-    |},
-    "inner": {|
-        type: "inner",
+    "atom": {|
+        type: "atom",
+        family: Atom,
         mode: Mode,
         loc?: ?SourceLocation,
         value: string,
@@ -192,38 +171,27 @@ type ParseNodeTypes = {
         loc?: ?SourceLocation,
         value: string,
     |},
-    "op-token": {|
-        type: "op-token",
-        mode: Mode,
-        loc?: ?SourceLocation,
-        value: string,
-    |},
-    "open": {|
-        type: "open",
-        mode: Mode,
-        loc?: ?SourceLocation,
-        value: string,
-    |},
-    "punct": {|
-        type: "punct",
+    "spacing": {|
+        type: "spacing",
         mode: Mode,
         loc?: ?SourceLocation,
         value: string,
     |},
-    "rel": {|
-        type: "rel",
+    "textord": {|
+        type: "textord",
         mode: Mode,
         loc?: ?SourceLocation,
         value: string,
     |},
-    "spacing": {|
-        type: "spacing",
+    // These "-token" types don't have corresponding HTML/MathML builders.
+    "accent-token": {|
+        type: "accent-token",
         mode: Mode,
         loc?: ?SourceLocation,
         value: string,
     |},
-    "textord": {|
-        type: "textord",
+    "op-token": {|
+        type: "op-token",
         mode: Mode,
         loc?: ?SourceLocation,
         value: string,
@@ -599,6 +567,40 @@ export function checkNodeType<NODETYPE: NodeType>(
     return null;
 }
 
+/**
+ * Asserts that the node is of the given type and returns it with stricter
+ * typing. Throws if the node's type does not match.
+ */
+export function assertAtomFamily(
+    node: ?AnyParseNode,
+    family: Atom,
+): ParseNode<"atom"> {
+    const typedNode = checkAtomFamily(node, family);
+    if (!typedNode) {
+        throw new Error(
+            `Expected node of type "atom" and family "${family}", but got ` +
+            (node ?
+                (node.type === "atom" ?
+                    `atom of family ${node.family}` :
+                    `node of type ${node.type}`) :
+                String(node)));
+    }
+    return typedNode;
+}
+
+/**
+ * Returns the node more strictly typed iff it is of the given type. Otherwise,
+ * returns null.
+ */
+export function checkAtomFamily(
+    node: ?AnyParseNode,
+    family: Atom,
+): ?ParseNode<"atom"> {
+    return node && node.type === "atom" && node.family === family ?
+        node :
+        null;
+}
+
 /**
  * Returns the node more strictly typed iff it is of the given type. Otherwise,
  * returns null.
@@ -618,7 +620,7 @@ export function assertSymbolNodeType(node: ?AnyParseNode): SymbolParseNode {
  * returns null.
  */
 export function checkSymbolNodeType(node: ?AnyParseNode): ?SymbolParseNode {
-    if (node && GROUPS.hasOwnProperty(node.type)) {
+    if (node && (node.type === "atom" || NON_ATOMS.hasOwnProperty(node.type))) {
         // $FlowFixMe
         return node;
     }
diff --git a/src/symbols.js b/src/symbols.js
index 44569888..badc05dd 100644
--- a/src/symbols.js
+++ b/src/symbols.js
@@ -24,20 +24,25 @@ type Font = "main" | "ams";
 // types for raw text tokens, and we want to avoid conflicts with higher-level
 // `ParseNode` types. These `ParseNode`s are constructed within `Parser` by
 // looking up the `symbols` map.
-export const GROUPS = { // Set of all the groups.
-    "accent-token": 1,
+export const ATOMS = {
     "bin": 1,
     "close": 1,
     "inner": 1,
-    "mathord": 1,
-    "op-token": 1,
     "open": 1,
     "punct": 1,
     "rel": 1,
+};
+export const NON_ATOMS = {
+    "accent-token": 1,
+    "mathord": 1,
+    "op-token": 1,
     "spacing": 1,
     "textord": 1,
 };
-export type Group = $Keys<typeof GROUPS>;
+
+export type Atom = $Keys<typeof ATOMS>;
+export type NonAtom = $Keys<typeof NON_ATOMS>
+export type Group = Atom | NonAtom;
 type CharInfoMap = {[string]: {font: Font, group: Group, replace: ?string}};
 
 const symbols: {[Mode]: CharInfoMap} = {
diff --git a/src/utils.js b/src/utils.js
index d357b643..154138aa 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -127,12 +127,7 @@ const isCharacterBox = function(group: AnyParseNode): boolean {
     // These are all they types of groups which hold single characters
     return baseElem.type === "mathord" ||
         baseElem.type === "textord" ||
-        baseElem.type === "bin" ||
-        baseElem.type === "rel" ||
-        baseElem.type === "inner" ||
-        baseElem.type === "open" ||
-        baseElem.type === "close" ||
-        baseElem.type === "punct";
+        baseElem.type === "atom";
 };
 
 export const assert = function<T>(value: ?T): T {


Test patch:
diff --git a/test/katex-spec.js b/test/katex-spec.js
index 92653fd8..5b5c530e 100644
--- a/test/katex-spec.js
+++ b/test/katex-spec.js
@@ -71,7 +71,8 @@ describe("A bin parser", function() {
 
         for (let i = 0; i < parse.length; i++) {
             const group = parse[i];
-            expect(group.type).toEqual("bin");
+            expect(group.type).toEqual("atom");
+            expect(group.family).toEqual("bin");
         }
     });
 });
@@ -96,7 +97,8 @@ describe("A rel parser", function() {
             if (group.type === "mclass") {
                 expect(group.value.mclass).toEqual("mrel");
             } else {
-                expect(group.type).toEqual("rel");
+                expect(group.type).toEqual("atom");
+                expect(group.family).toEqual("rel");
             }
         }
     });
@@ -114,7 +116,8 @@ describe("A punct parser", function() {
 
         for (let i = 0; i < parse.length; i++) {
             const group = parse[i];
-            expect(group.type).toEqual("punct");
+            expect(group.type).toEqual("atom");
+            expect(group.family).toEqual("punct");
         }
     });
 });
@@ -131,7 +134,8 @@ describe("An open parser", function() {
 
         for (let i = 0; i < parse.length; i++) {
             const group = parse[i];
-            expect(group.type).toEqual("open");
+            expect(group.type).toEqual("atom");
+            expect(group.family).toEqual("open");
         }
     });
 });
@@ -148,7 +152,8 @@ describe("A close parser", function() {
 
         for (let i = 0; i < parse.length; i++) {
             const group = parse[i];
-            expect(group.type).toEqual("close");
+            expect(group.type).toEqual("atom");
+            expect(group.family).toEqual("close");
         }
     });
 });