Rewrite spacing commands and \mod as macros #1156

edemaine · 2018-02-13T14:28:22Z

Fix #1130 by defining \!, \negthinspace, \,, \thinspace, \:, \medspace, \;, \thickspace, \negmedspace, \negthickspace, \enspace, \enskip, \qquad, \quad as macros. Some of these are new. Also, we now use the AMS definitions of spaces, which are slightly different (e.g., using 1mu = 1/18em instead of .05555em, and working in text mode instead of just math mode).

Fix #1036 by defining a new SpaceNode in mathMLTree that recognizes all special space amounts from https://www.w3.org/TR/2000/WD-MathML2-20000328/chapter6.html

Fix KaTeX#1130 by defining `\!`, `\negthinspace`, `\,`, `\thinspace`, `\:`, `\medspace`, `\;`, `\thickspace`, `\negmedspace`, `\negthickspace`, `\enspace`, `\enskip`, `\qquad`, `\quad` as macros. Fix KaTeX#1036 by defining a new `SpaceNode` in mathMLTree that recognizes all special space amounts from https://www.w3.org/TR/2000/WD-MathML2-20000328/chapter6.html

codecov · 2018-02-13T14:33:38Z

Codecov Report

Merging #1156 into master will increase coverage by 0.72%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1156      +/-   ##
==========================================
+ Coverage   81.56%   82.29%   +0.72%     
==========================================
  Files          78       77       -1     
  Lines        4259     4242      -17     
  Branches      734      732       -2     
==========================================
+ Hits         3474     3491      +17     
+ Misses        670      649      -21     
+ Partials      115      102      -13

Impacted Files	Coverage Δ
src/defineFunction.js	`93.75% <ø> (ø)`	⬆️
src/functions.js	`100% <ø> (ø)`	⬆️
src/symbols.js	`100% <ø> (ø)`	⬆️
src/functions/symbolsSpacing.js	`64.7% <0%> (-35.3%)`	⬇️
src/buildCommon.js	`89.78% <100%> (ø)`	⬆️
src/macros.js	`87.9% <100%> (+0.97%)`	⬆️
src/functions/kern.js	`81.25% <100%> (+9.02%)`	⬆️
src/functions/operatorname.js	`42.42% <100%> (ø)`	⬆️
src/mathMLTree.js	`75.43% <78.78%> (+4.47%)`	⬆️
src/SourceLocation.js	`100% <0%> (+11.11%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 978148e...200ded1. Read the comment docs.

edemaine · 2018-02-13T16:33:14Z

src/mathMLTree.js

+     */
+    toMarkup(): string {
+        if (this.character) {
+            return this.character;


I'm a little confused about the correct markup here. Perhaps it should be `<mtext>${this.character}</mtext>`, and then we need to remove this <mtext> wrapper if it gets included in an <mo> or other token element?

I'm not sure how to do this either. In the past I've look at what MathJax outputs, but <mspace width="thinmathspace" /> doesn't seem ideal. Is it bad if we include <mtext> nodes inside <mo> or other element types?

edemaine · 2018-02-13T16:50:05Z

test/__snapshots__/mathml-spec.js.snap

@@ -142,7 +141,7 @@ exports[`A MathML builder should output \\limsup_{x \\rightarrow \\infty} correc
      <msub>
        <mo>
          <mi mathvariant="normal">
-            lim sup
+            lim&amp;ThinSpace;sup


This extra escaping is a bug, caused by operatorname's mathmlBuilder creating a mathMLTree.TextNode, which escapes its text. I'm not quite sure how to fix it, other than switching to the Unicode representations...

We're definitely doing something wrong here. In toNode we call document.createTextNode which expects to be passed an escaped string, yet we're passing this.text. I feel like we should maybe be doing the escaping within the constructor and then we could add an escape as an optional second parameter with the default being true.

kevinbarabash · 2018-02-18T14:45:02Z

@edemaine looks like some of the screenshots changed. Could you add updated screenshots when you have some time?

edemaine · 2018-02-18T18:36:55Z

@kevinbarabash Done. This has revealed more bugs, which I'll take a look at later.

kevinbarabash · 2018-05-20T00:11:13Z

@edemaine what kind of bugs did this reveal?

edemaine · 2018-05-21T15:52:43Z

@kevinbarabash I meant bugs in this PR. I just merged with the many changes on master, and will take another look at this. CircleCI's screenshot diffs will help too! Here are the diffs, for the record:

$fractiontest-chrome-diff$

Specifically, Dots and NegativeSpaceBetweenRel seem ~~broken~~very different, and I should check whether the ArrayMode tweaks are for better or worse.

edemaine · 2018-05-24T15:48:43Z

I finally took a look at these. It turns out that the big changes are actually bug fixes! NegativeSpaceBetweenRel was particularly broken -- the negative space wasn't rendering! Not sure how we missed that (perhaps a side-effect of the line-breaking code?). But I'm glad this fixes it. And Dots was similarly missing a negative space (\!). Here are texcmp outputs against the new behavior, confirming they are correct:

ArrayMode has minor changes, and the texcmp (below) is so far to not be very revealing. But given that the other changes are substantial improvements, I'd say we should merge.

kevinbarabash · 2018-05-24T18:44:42Z

@edemaine there are some flow errors that need resolving before we can merge.

edemaine · 2018-05-25T14:00:53Z

@kevinbarabash Oops, fixed!

kevinbarabash · 2018-05-25T14:31:03Z

The Mod screenshots changed quite a bit. Also the CJK rows in Unicode-chrome.png have disappeared.

edemaine · 2018-05-25T18:07:51Z

@kevinbarabash Thanks for spotting those -- not sure how I missed Mod in particular.

c6f3a7a recompiles the Unicode-chrome screenshot. This was caused some unfortunate nondeterminism in the screenshotter loading the supporting fonts, as evidenced by the screenshot tests actually passing with the wrong image! But now it's the correct image, phew.

The problem with the Mod test is that the various mod operators created manual elements of class mspace with various subclasses to do the spacing. It seemed quite a bit easier to start over completely, and reimplement the mod operators (amsmath versions) via macros. It was quite easy, and cleans up the code further. This is 2bb10f7 . And now Mod doesn't differ at all!

ModScript seems to have an issue though, which I'm still looking at...

edemaine · 2018-05-25T19:05:40Z

Fixed ModScript; I just had to remember what \nonscript meant again. 😄 So glad we have \mathchoice now! But I might try to implement \nonscript in a future PR.

texcmp for a slightly tweaked (single column, left justified) ModScript test:

For comparison, here's what the texcmp for master branch looks like:

Our spaces seem smaller than LaTeX's in both cases, but the changes from this PR seem like a step in the right direction. (And the script fonts are so different in LaTeX vs. KaTeX that it's difficult to know the precise cause. Maybe even ems are wrong in our script font...)

kevinbarabash

LGTM. Thanks for also improving the MathML output to include all of those specific spacing characters.

kevinbarabash · 2018-05-29T12:26:43Z

src/functions.js

@@ -24,7 +24,6 @@ import "./functions/lap";
 import "./functions/math";
 import "./functions/mathchoice";
 import "./functions/mclass";
-import "./functions/mod";


kevinbarabash · 2018-05-29T12:28:33Z

src/mathMLTree.js

+        if (width >= 0.05555 && width <= 0.05556) {
+            this.character = "&VeryThinSpace;";  // \u200a
+        } else if (width >= 0.1666 && width <= 0.1667) {
+            this.character = "&ThinSpace;";      // \u2009


kevinbarabash · 2018-05-29T12:30:29Z

src/mathMLTree.js

+        // See https://www.w3.org/TR/2000/WD-MathML2-20000328/chapter6.html
+        // for a table of space-like characters.  We consistently use the
+        // &LongNames; because Unicode does not have single characters for
+        // &ThickSpace; (\u2005\u200a) and all negative spaces.


I don't don't understand this comment. I don't see $LongNames; anywhere.

I meant   is an example of a long name between a & and ;, as opposed to the Unicode equivalent. See the table in Section 6.1.4. I could change to &CharacterNames; or just "character names" if that would be clearer.

edemaine · 2018-05-29T12:44:13Z

@kevinbarabash Thanks for the review!

edemaine commented Feb 13, 2018

View reviewed changes

Fix <mspace> rendering and add a test to catch it

6a09a4a

Update screenshots

06476ad

edemaine added the GH Review: needs-revision label Feb 18, 2018

Merge remote-tracking branch 'upstream/master' into spacingmacros

4c96da2

edemaine added 2 commits May 21, 2018 11:56

Wrap special space characters in <mtext>

c01be26

Update screenshots

e90809f

edemaine added 2 commits May 24, 2018 11:51

Merge remote-tracking branch 'upstream/master' into spacingmacros

3c929db

Fix MathML escaping behavior

9529a40

edemaine added GH Review: review-needed and removed GH Review: needs-revision labels May 24, 2018

kevinbarabash assigned edemaine May 24, 2018

kevinbarabash added GH Review: needs-revision and removed GH Review: review-needed labels May 24, 2018

edemaine added 2 commits May 25, 2018 09:55

Merge remote-tracking branch 'upstream/master' into spacingmacros

dbd6280

Fix flow typos

ffd2896

edemaine added GH Review: review-needed and removed GH Review: needs-revision labels May 25, 2018

edemaine removed their assignment May 25, 2018

edemaine added 2 commits May 25, 2018 13:47

Fix Unicode-chrome

c6f3a7a

Reimplement mod operators in terms of macros

2bb10f7

edemaine added 3 commits May 25, 2018 14:10

Rerun ModScript

176aaa3

Remove excess table entries (now already macros)

2748b32

Fix bmod

517b344

kevinbarabash approved these changes May 29, 2018

View reviewed changes

Merge branch 'master' into spacingmacros

200ded1

kevinbarabash added GH review: accepted and removed GH Review: review-needed labels May 29, 2018

kevinbarabash merged commit 417383e into KaTeX:master May 29, 2018

edemaine changed the title ~~Rewrite spacing commands as macros~~ Rewrite spacing commands and \mod as macros May 29, 2018

ylemkimon mentioned this pull request Aug 18, 2018

Release v0.10.0-rc #1634

Merged

1 task

snyk-bot mentioned this pull request Feb 23, 2020

[Snyk] Upgrade katex from 0.9.0 to 0.11.1 saurabharch/Rocket.Chat#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite spacing commands and \mod as macros #1156

Rewrite spacing commands and \mod as macros #1156

edemaine commented Feb 13, 2018 •

edited

Loading

codecov bot commented Feb 13, 2018 •

edited

Loading

edemaine Feb 13, 2018

kevinbarabash Feb 14, 2018

edemaine Feb 13, 2018

kevinbarabash Feb 14, 2018

kevinbarabash commented Feb 18, 2018

edemaine commented Feb 18, 2018

kevinbarabash commented May 20, 2018

edemaine commented May 21, 2018 •

edited

Loading

edemaine commented May 24, 2018

kevinbarabash commented May 24, 2018

edemaine commented May 25, 2018

kevinbarabash commented May 25, 2018

edemaine commented May 25, 2018 •

edited

Loading

edemaine commented May 25, 2018 •

edited

Loading

kevinbarabash left a comment

kevinbarabash May 29, 2018

kevinbarabash May 29, 2018

kevinbarabash May 29, 2018

edemaine May 29, 2018

edemaine commented May 29, 2018

Rewrite spacing commands and \mod as macros #1156

Rewrite spacing commands and \mod as macros #1156

Conversation

edemaine commented Feb 13, 2018 • edited Loading

codecov bot commented Feb 13, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinbarabash commented Feb 18, 2018

edemaine commented Feb 18, 2018

kevinbarabash commented May 20, 2018

edemaine commented May 21, 2018 • edited Loading

edemaine commented May 24, 2018

kevinbarabash commented May 24, 2018

edemaine commented May 25, 2018

kevinbarabash commented May 25, 2018

edemaine commented May 25, 2018 • edited Loading

edemaine commented May 25, 2018 • edited Loading

kevinbarabash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edemaine commented May 29, 2018

edemaine commented Feb 13, 2018 •

edited

Loading

codecov bot commented Feb 13, 2018 •

edited

Loading

edemaine commented May 21, 2018 •

edited

Loading

edemaine commented May 25, 2018 •

edited

Loading

edemaine commented May 25, 2018 •

edited

Loading