Add mhchem extension #1436

ronkok · 2018-06-16T22:26:18Z

This is a proposal more than it is a pull request. Or perhaps it’s a proof of concept. The proposed code allows KaTeX to interact with a proposed mhchem extension. Such a mhchem extension would be installed in a way similar to the auto-render extension or the copy-tex extension. That is, web site administrators would list the mhchem-for-katex.js file in the <head> of their page.

I invite comments. I expect that @edemaine can write a better helper function to recreate the argument to be passed to the extension. I’m sure that @mhchem can improve on the edits that I will suggest to the mhchem code.

It’s pretty simple, really. mhchem expands a macro. So I’ve written a suggested wrapper around the mhchem code that exposes a object with a single method: mhchem.expand(str: string, stateMachine: "ce"|"pu") : {expansion: string, errorMsg: string}.

~~The code in this PR checks for existence of the object, then calls the function.~~

General revision (Nov 3): This PR adds support for the \ce and \pu functions from the mhchem package. However, this PR makes no revisions to any core KaTeX files. Instead, it exploits the fact that KaTeX exposes the defineMacro function. All the mhchem code is a separate file. So web page administrators will be able to choose whether or not to include mhchem as an extension.

As in the original version of this PR, the code is mostly a copy of the mhchem package for MathJax. I still have made only four modifications, as described just below.

ronkok · 2018-06-16T22:26:56Z

To test this PR, I have made four modifications to the code of mhchem.js, for coordination with KaTeX:

First, the MathJax wrapper is replaced with a wrapper that includes callbacks from KaTeX .__defineMacro:

katex.__defineMacro("\\ce", function(context) {
  return mhchem.expand(context.consumeArgs(1)[0], "ce")
});

katex.__defineMacro("\\pu", function(context) {
  return mhchem.expand(context.consumeArgs(1)[0], "pu");
});

//  Needed for \bond for the ~ forms
katex.__defineMacro("\\tripledash", "\\vphantom{-}\\raisebox{2mu}{$\\mkern2mu"
  + "\\tiny\\text{-}\\mkern1mu\\text{-}\\mkern1mu\\text{-}\\mkern2mu$}");

// Main object
var mhchem = (function () {
  var chemParse = function (tokens, stateMachine) {
    // Recreate the argument string from KaTeX's array of tokens.
    var str = "";
    for (var i = tokens.length - 1; i >= 0; i--) {
        str += tokens[i].text;
        if (tokens[i].text.charAt(0) === "\\") {
            str += " ";  // Separate functions from text.
        }
        // Eliminate spaces between any function and {
        str = str.replace(/\s(?=\{)/g, "");
    }
      var tex = texify.go(mhchemParser.go(str, stateMachine));
      return tex;
  };
…
… existing code, mostly unchanged
…

return {
  expand: chemParse,
}

})();

Second, I replaced \rlap and \llap with \mathrlap and \mathllap.

Third, I modified four lines of code in order to change \raise to \raisebox.

      "~-": "{\\mathrlap{\\raisebox{-.1em}{$-$}}\\raisebox{.1em}{$\\tripledash$}}",
      "~=": "{\\mathrlap{\\raisebox{-.2em}{$-$}}\\mathrlap{\\raisebox{.2em}{$\\tripledash$}}-}",
      "~--": "{\\mathrlap{\\raisebox{-.2em}{$-$}}\\mathrlap{\\raisebox{.2em}{$\\tripledash$}}-}",
      "-~-": "{\\mathrlap{\\raisebox{-.2em}{$-$}}\\mathrlap{\\raisebox{.2em}{$-$}}\\tripledash}",

Fourth, I simplified the code for the reaction arrows, since KaTeX provides a full set of extensible arrows and work around hacks are not needed.

    'arrow': function (buf) {
        buf.rd = texify.go2(buf.rd);
        buf.rq = texify.go2(buf.rq);
        var arrow = texify.arrows[buf.r];
    if (buf.rq) { arrow += "[{" + buf.rq + "}]"; }
    if (buf.rd) {
      arrow += "{" + buf.rd + "}";
    } else {
      arrow += "{}";
    }
        return arrow;
      },
      'operator': function (buf) { return texify.operators[buf.kind]; }
    },
    arrows: {
      "->": "\\xrightarrow",
      "\u2192": "\\xrightarrow",
      "\u27F6": "\\xrightarrow",
      "<-": "\\xleftarrow",
      "<->": "\\xleftrightarrow",
      "<-->": "\\xrightleftarrows",
      "<=>": "\\xrightleftharpoons",
      "\u21CC": "\\xrightleftharpoons",
      "<=>>": "\\xrightequilibrium",
      "<<=>": "\\xleftequilibrium"

I’ve done a local build with those changes and the code in this PR. With that, I can reproduce every formula on the mhchem Manual page.

The code here is only a suggestion. @mhchem owns this work and he can adopt it, change it, or discard it just as he chooses.

codecov · 2018-06-16T22:29:34Z

Codecov Report

❗ No coverage uploaded for pull request base (master@cda184b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1436   +/-   ##
=========================================
  Coverage          ?   93.67%           
=========================================
  Files             ?       80           
  Lines             ?     4666           
  Branches          ?      813           
=========================================
  Hits              ?     4371           
  Misses            ?      261           
  Partials          ?       34

Flag	Coverage Δ
#screenshotter	`88.62% <ø> (?)`
#test	`85.04% <ø> (?)`

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 cda184b...de812e1. Read the comment docs.

kevinbarabash · 2018-06-16T23:08:35Z

@ronkok this is super cool! I'm glad that you were able to adapt the MathJax extension without too much work. Could you post some example renderings?

ronkok · 2018-06-16T23:18:08Z

Screenshots:

mhchem · 2018-06-17T11:12:45Z

This is splendid!

What I see here, seems very reasonable to me. So, I would leave the creation of the first version up to you. You decide, where to place the files, how to include them, etc. After that, I'd like to have control over the mhchem core for fixes and updates. You decide, how you would like to have that. Maybe a file at KaTeX/contrib/mhchem/mhchem.js where I can file pull requests? Or a completely separate repository? Maybe with your standard minification, maybe with mine (that is specially tweaked). You decide. I don't think, I will need a veto.

I am looking forward to having mhchem support in KaTeX and working with you.

ronkok · 2018-06-17T13:34:24Z

@mhchem That sounds very reasonable to me. Let's give @kevinbarabash a chance to review this PR. If he approves, then I'll work up a PR to create an mhchem extension arranged in a manner similar to copy-tex. After I've written the first version of the extension, you can then maintain it.

ylemkimon · 2018-06-19T08:47:01Z

As most of macro expansions don't need to access KaTeX internals and defineMacro is already exposed, I think we can easily add plugins like mhchem!

kevinbarabash · 2018-06-23T05:10:34Z

@mhchem I think having things live at KaTeX/contrib/mhchem/mhchem.js makes sense. @ronkok can you include the modified mhchem.js you created as part of this PR since the current code isn't much use without it? It would be nice to have a screenshot test, but that can be a separate PR since it will require updating the screenshotter to to load the extension.

ylemkimon · 2018-06-24T07:02:21Z

src/macros.js

+
+defineMacro("\\pu", function(context) {
+    return mhChemExpansion(context, "pu");
+});


I think macro definitions should also be in contrib too.

I think macro definitions should also be in contrib too.

I would like that, too, so let's discuss what would be required to make that happen. At present, KaTeX exposes __defineMacro: defineMacro, but that only enables one to define a macro in the KaTeX macro language. KaTeX does not support branching macro code such as \if … \else … \fi, so the complexity of mhchem is out of reach by that method.

I needed a way to (1) capture the functions \ce and \pu, (2) reconstruct the string in the argument to those functions, (3) pass the string to the external mhchem extension, and (4) receive the expanded macro or an error message. This PR does that.

Maybe I should have added an abstraction layer that would enable other similar extensions to do something similar. Such an abstraction layer would expose a _defineComplexMacro callback and would enable complexMacro.expand(argString: string, functionName: string) : {expansion: string, errorMsg: string} Then I could have used that exposed capability to define \ce and \pu in the contrib folder

Maybe that's what's people mean when they talk of a plugin architecture. I don't know. I'm out of my depth here. There are different ways to do this and I'm probably not the best person to do decide which way is best. That's why I opened this PR by describing it as a proof of concept, not a finished product. I would welcome instruction. If someone wanted to step in and define that proposed callback function, I would be fine with that, too.

__defineMacro works just like defineMacro. Its second argument can be a function, as above. Is there something more you need?

defineMacro. Its second argument can be a function, as above.

I might be able to work with that. Let me get back to you.

Update: I can indeed pass a function in _defineMacro, just as @edemaine suggests. In fact, no modifications are necessary to macros.js at all. It works fine.

Before, instructions to the web site administrator were to place the reference to mhchem.js in the <head> of the HTML page. As now proposed, they will have to do that, and also write the following into their KaTeX rendering options:

macros: { "\\ce": function(context) {return mhchem.expand(context.consumeArgs(1)[0], "ce")}, "\\pu": function(context) {return mhchem.expand(context.consumeArgs(1)[0], "pu")}, "\\tripledash": "\\vphantom{-}\\raisebox{2mu}{$\\mkern2mu\\tiny\\text{-}\\mkern1mu\\text{-}\\mkern1mu\\text{-}\\mkern2mu$}" }

In exchange for that extra installation complexity, we get cleaner code in macros.js. Is that a good trade off?

I don't like it. In particular, the \\tripledash part is part of internals a user should not never see. Also, the other definitions do not make me happy. What will happen when you will change the internals some time in the future?

I don't like it much either. And now that I look back, I think that @ylemkimon wanted to suggest something to move \ce definition into contrib, not into a rendering option. But I don't know how to accomplish such a thing. If anyone can tell me how, I'll take a crack at it.

Otherwise, I think we are better off to keep the code in macros.js.

Is it impossible to do following in the extension:

const mhChemExpansion = <omitted>; katex.__defineMacro("\\ce", function(context) { return mhChemExpansion(context, "ce"); }); katex.__defineMacro("\\pu", function(context) { return mhChemExpansion(context, "pu"); });

?

One minor thing. I would prefer the all-lowercase spelling of mhchem over mhChem.

@ylemkimon Thank you for the instruction!

ronkok · 2018-06-25T16:38:39Z

The latest commit moves all the code from macros.js to mhchem.js. Now this PR makes no changes whatsoever to core KaTeX. All the code is in the extension. Also, no rendering options need be defined by a web site administrator. All the macros are defined in mhchem.js.

Thank you to @ylemkimon and @edemaine for instruction on how to better arrange things. It's a better PR now.

ronkok · 2018-06-25T17:55:37Z

The KaTeX linter says that nearly every line of mhchem.js has a problem. Acturally, mhchem.js is written to style standards that are good, but different than the KaTeX standards. Is there a way to modify the eslint script that makes an exception for the mhchem extension?

edemaine · 2018-06-25T18:24:50Z

@ronkok Put /* eslint-disable */ at the top of the file.

edemaine · 2018-06-25T18:32:35Z

A couple other quick comments:

Why is mhchem.min.js in the repo? Presumably it should instead be generated?
It seems that mhchem is licensed under Apache 2.0, while KaTeX is licensed under MIT. I think some care is needed here, though I'm not an expert on the Apache license. Maybe @mhchem can comment.

ronkok · 2018-06-25T18:47:24Z

Why is mhchem.min.js in the repo?

I'll go further. There is no particular reason for mhchem.js to be in the KaTeX repo. @mhchem could take possession of the mhchem.js file, keep it somewhere in https://github.com/mhchem/ and push it to a CDN. It would run, right now, with no revision needed to KaTeX.

In the KaTeX repo, there would still be a mhchem entry in contrib, but it would contain only a README page that pointed to mhchem.min.js on its CDN. I'd also add \ce and \pu to the KaTeX function support page.

In that arrangement, there are no lint issues, no licensing conflicts, and @mhchem gets to keep control over his code.

Comments?

edemaine · 2018-06-25T19:18:28Z

I would lean more toward just the main mhchem module being "off site", and just the KaTeX-related tweaks being here. I was wondering whether mhchem.js could be obtained from a Git submodule. This would require the upstream copy to have a different or more tweakable loading strategy.

Personally, I serve KaTeX files from my own server, not CDN. (Debatable whether that's a good idea.) This would be one reason that it'd be more convenient to have things in the KaTeX distribution, but avoiding the main duplication would be nice.

ronkok · 2018-06-25T20:22:38Z

main mhchem module being "off site", and just the KaTeX-related tweaks being here.

The previously existing version of mhchem.js is a single file that contains a fair amount of MathJax-specific glue code. It turned out to be fairly easy to remove those parts and replace with code that ties into KaTeX's defineMacro(). But there were no "main mhchem" files that contain non-MathJax material.

Personally, I serve KaTeX files from my own server.

@mhchem makes the mhchem.js file available in the https://github.com/mhchem/ repo. It's just as easy to download them from there as from here. And we could write a link into the KaTeX contrib/mhchem README page .

mhchem · 2018-06-25T21:13:31Z

But there were no "main mhchem" files that contain non-MathJax material.

Which could be created if need be.

To repeat myself: It's your decision where the files live. I don't have a preference. (As long as I can keep the behavior of mhchem for MathJax and mhchem for KaTeX aligned, I am happy.)

ronkok · 2018-06-25T22:02:11Z

It's your decision where the files live.

So I will confess a personal interest. We now have a mhchem.js that works nicely with KaTeX. We've completed the part of the job at which I am any good. As soon as we move away from Javascript or CSS and into infrastructure, then we move into areas of which I know very little. And so my personal interest is that any path is okay, so long as someone else does it.

kevinbarabash · 2018-06-26T02:17:55Z

But there were no "main mhchem" files that contain non-MathJax material.

Which could be created if need be.

That seems like best path forward. If this module was published then it could be required by the MathJax plugin and the KaTeX extension. @mhchem how much would is involved in this?

kevinbarabash · 2018-11-20T02:07:47Z

~~@ylemkimon any idea as to why circleci is complaining about the $CIRCLE_PROJECT_USERNAME != "Khan" in the test job? I can't find where this check is being done. 😞~~

I found it. I see you've addressed it in #1781. I'll review that right now.

kevinbarabash · 2018-11-20T14:26:40Z

#1781 has been reviewed and merged. Rebasing this PR should fix the circleci: test issue.

codecov-io · 2018-11-20T17:05:48Z

Codecov Report

Merging #1436 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1436   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files          80       80           
  Lines        4653     4653           
  Branches      807      807           
=======================================
  Hits         4358     4358           
  Misses        261      261           
  Partials       34       34

Flag	Coverage Δ
#screenshotter	`88.85% <ø> (ø)`	⬆️
#test	`84.99% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
src/svgGeometry.js	`100% <ø> (ø)`	⬆️

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 3dfd17d...cc429e2. Read the comment docs.

kevinbarabash · 2018-11-24T23:48:09Z

@ronkok thanks for all of the changes. I'm excited about support for mhchem and even more excited that you were able to do it as an extension. Even though mhchem is a fairly standard LaTeX package I think this PR will serve as a good example how people might extend KaTeX is less standard ways via extensions.

ronkok · 2018-11-25T00:03:42Z

@kevinbarabash You are very welcome.

This, I think, is a good pause point. I have submitted PR's that supported 140 TeX control words, supported 600 Unicode characters, fixed 9 rendering bugs (not counting the ones I created), and added working examples to the documentation. I think I've done most of what I came here to do. Let me know if you find bugs in the code I submitted. Beyond that, my involvement henceforward will be much less that it has been the past 18 months.

To you, to @edemaine and @ylemkimon, I've appreciated the support and encouragement. You have a great library going here.

kevinbarabash · 2018-11-25T00:24:01Z

@ronkok thanks for all of your many contributions and thanks for the heads up that you'll be moving on to other things. I appreciate your offer to support the code that you've submitted. If you end up using KaTeX in any future projects, I'd love to hear what you're doing with it. 🙂

ylemkimon · 2018-11-26T06:03:55Z

@ronkok Thank you for your great contributions! It was a pleasure to work with you 😄

mhchem · 2018-11-27T00:25:23Z

@ronkok Hooray! Thank you so much, that was quite some work! Thanks!

Okay, I'll take it from here. If I update mhchem, I will update it for both, MathJax and KaTeX, from now on. The setup looks straight forward for me (I'd modify mhchem.js, file a pull request and your workflow does minification etc.).

Once again, Thank you!

* A proposal for support of mhchem * Create extension * Add index * Remove arrow function * Fix lint errors in macros.js * Remove mhchem from build * Move code from macros.js to mhchem.js * Add minified mhchem * Remove arrow head mods. Disable eslint. * Fix space handling * Update code to mhchem v3.3.0 * Tweak arrows to reduce diff * Raise \tripledash * Edit docs and webpack * Update package.json * Remove wrap. Remove manual. * Correct second brush stroke in \xrightleftarrows * Added patch file * Add import line * Add mhchem to eslint ignore

ronkok · 2020-06-20T22:40:41Z

@kevinbarabash 1½ years ago, you said you would like to hear if I used KaTeX in any future projects. Well, now I have. I put up https://hurmet.app/ a few days ago and I'm quite happy with it.

kevinbarabash · 2020-08-02T03:04:40Z

@ronkok neat! It reminds me of juypter notebooks. I like how it allows you to computations on taels in a very succinct way and over course it's nice to see nicely typeset math. 🙂

A proposal for support of mhchem

511ae09

ronkok added 5 commits June 23, 2018 08:40

Merge branch 'master' into mhchem

5590670

Create extension

22b3c11

Add index

ad42566

Remove arrow function

5944d47

Fix lint errors in macros.js

da61d11

ylemkimon reviewed Jun 24, 2018

View reviewed changes

ronkok added 2 commits June 24, 2018 15:14

Remove mhchem from build

100b4a7

Move code from macros.js to mhchem.js

1d9fd5a

Add minified mhchem

a16531e

ronkok changed the title ~~A proposal for support of mhchem~~ Add mhchem extension Jun 25, 2018

ronkok mentioned this pull request Jun 25, 2018

mhchem extension support #50

Closed

ronkok mentioned this pull request Nov 20, 2018

Raise \tripledash mhchem/MathJax-mhchem#7

Closed

Merge branch 'master' into mhchem

3414c7c

ronkok added 7 commits November 20, 2018 09:37

Remove wrap. Remove manual.

e1059d1

Correct second brush stroke in \xrightleftarrows

8e18bbf

Added patch file

178a91d

Merge branch 'master' into mhchem

421b7ec

Add import line

5695411

Merge branch 'mhchem' of https://github.com/ronkok/KaTeX into mhchem

b539d7b

Add mhchem to eslint ignore

047ca55

kevinbarabash approved these changes Nov 24, 2018

View reviewed changes

Merge branch 'master' into mhchem

cc429e2

kevinbarabash merged commit 64745b5 into KaTeX:master Nov 24, 2018

ronkok deleted the mhchem branch November 25, 2018 00:11

kevinbarabash added GH review: accepted and removed GH Review: review-needed labels Nov 25, 2018

OnyxAzryn mentioned this pull request Mar 7, 2019

mhchem for Katex notable/notable#434

Closed

pierre-24 mentioned this pull request Aug 19, 2019

KaTeX: add mchem zestedesavoir/zmarkdown#359

Closed

StaloneLab mentioned this pull request Aug 30, 2019

Allow to pass a custom KaTeX renderer remarkjs/remark-math#34

Closed

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

Add mhchem extension #1436

Add mhchem extension #1436

Conversation

ronkok commented Jun 16, 2018 • edited Loading

ronkok commented Jun 16, 2018 • edited Loading

codecov bot commented Jun 16, 2018 • edited Loading

Codecov Report

kevinbarabash commented Jun 16, 2018

ronkok commented Jun 16, 2018

mhchem commented Jun 17, 2018

ronkok commented Jun 17, 2018 • edited Loading

ylemkimon commented Jun 19, 2018

kevinbarabash commented Jun 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edemaine Jun 24, 2018 • edited Loading

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

ylemkimon Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronkok commented Jun 25, 2018

ronkok commented Jun 25, 2018

edemaine commented Jun 25, 2018

edemaine commented Jun 25, 2018

ronkok commented Jun 25, 2018 • edited Loading

edemaine commented Jun 25, 2018

ronkok commented Jun 25, 2018

mhchem commented Jun 25, 2018

ronkok commented Jun 25, 2018 • edited Loading

kevinbarabash commented Jun 26, 2018 • edited Loading

kevinbarabash commented Nov 20, 2018 • edited by ylemkimon Loading

kevinbarabash commented Nov 20, 2018

codecov-io commented Nov 20, 2018 • edited Loading

Codecov Report

kevinbarabash commented Nov 24, 2018

ronkok commented Nov 25, 2018

kevinbarabash commented Nov 25, 2018

ylemkimon commented Nov 26, 2018

mhchem commented Nov 27, 2018

ronkok commented Jun 20, 2020

kevinbarabash commented Aug 2, 2020

ronkok commented Jun 16, 2018 •

edited

Loading

ronkok commented Jun 16, 2018 •

edited

Loading

codecov bot commented Jun 16, 2018 •

edited

Loading

ronkok commented Jun 17, 2018 •

edited

Loading

kevinbarabash commented Jun 23, 2018 •

edited

Loading

edemaine Jun 24, 2018 •

edited

Loading

ylemkimon Jun 25, 2018 •

edited

Loading

ronkok commented Jun 25, 2018 •

edited

Loading

ronkok commented Jun 25, 2018 •

edited

Loading

kevinbarabash commented Jun 26, 2018 •

edited

Loading

kevinbarabash commented Nov 20, 2018 •

edited by ylemkimon

Loading

codecov-io commented Nov 20, 2018 •

edited

Loading