executablebooks / markdown-it-amsmath Goto Github PK

For the default rule name, it would be great if we could have some consistency with dollarmath package? Maybe math_block_amsmath? Or simply make it a math_block and add some metadata to the token to indicate that it is AMS?

It would be great to have consistency between:

$$
\begin{equation}
Ax = b
\end{equation}
$$ (math)

and

\begin{equation}
\label{math}
Ax = b
\end{equation}

and

```{equation}
:label: math
Ax = b
```

and

$$
Ax = b
$$ (math)

In terms of downstream packages not having to know the intricacies of how the math was parsed, this would mean the tokens in all cases would be something close to:

token.type = 'math_block'
token.content = 'Ax = b' || '\begin{...'
token.meta = {amsmath: true, label: 'math', numbered: true}

Maybe optionally strip out equation environments and parse simple label information. In the future as well, as packages may want to label things on their own.

Describe the bug

Mathjax, which is the default parser in jupyter notebooks is much more permissive when parsing environments than the list here:
https://github.com/executablebooks/markdown-it-amsmath/blob/main/src/index.ts#L53

For example cases is required to be in an ams environment, but mathjax picks up the \begin and \end tags fine, and then wraps it in a \begin{equation*} if it is not in the list above.

When using this to parse notebooks written in jupyter, many people use this shortcut and so the myst parsing fails, and often gets very messy because of underscores in the math being matched outside of the math environment.

Reproduce the bug

Parse:

\begin{cases}
0& \text{if $r-j$ is odd},\\
r!\,(-1)^{(r-j)/2}& \text{if $r-j$ is even}.
\end{cases}

In Jupyter (using mathjax)

List your environment

No response

I believe that we should first html-escape the content before passing to the renderer:

https://github.com/executablebooks/markdown-it-amsmath/blob/master/src/index.ts#L35

There could be an optional parameter to turn off this functionality, but if we are letting users override the renderer I think we should default to doing so safely.

Is your feature request related to a problem? Please describe.

Looking at the default renderer here: https://github.com/executablebooks/markdown-it-amsmath/blob/master/src/index.ts#L45

It is currently <section class="amsmath"><eqn>{content}</eqn></section>.

Reading the HTML spec, sections require a header in most situations and generally have more content than a single element. Also note that eqn is not a valid tag and I don;t think we should be providing this as the default.

Describe the solution you'd like

I would propose we use a div with a class for the default.

Either:

<div class="amsmath">{content}</div>

Or simply:

<div class="math amsmath">{content}</div>

Which might work better with dollarmath (math inline and math display) and keep css classes simple/shared.

Describe alternatives you've considered

This can be overridden through the renderer, however, the default should provide good basics and valid html - sticking to semantic rules if possible!

Additional context

In the previous js implementation of this, we were adding a \[ before and after the div, this allowed the default script of mathjax and katex to render when this HTML was output. The class chosen was math rather than amsmath because from a rendering perspective there isn't a difference, so class selection seemed easier with that choice.

I was a bit confused first looking at the interface what the options in both this package and dollarmath that the options we pass in correspond to the user defined renderer function, rather than the package. amsmath does not modify the options, and then calls the renderer with a specific call signature. I feel like these should be the responsibility of the optional renderer, not the package.

export interface IOptions {
  // the render function to use
  renderer?: (content: string, options?: { [key: string]: any }) => string
  // options to parse to the render function
  options?: { [key: string]: any }
}

I would suggest simplifying this interface and encouraging users to deal with the options themselves. Passing them to the package gives me the impression that the library is doing something with them. For example, I am telling amsmath to throwOnError rather than katex.

For example:

const katexOptions = { throwOnError: false, displayMode: true }
const renderer = (mathString, maybeTheToken) => katex.renderToString(mathString, katexOptions)
const md = window.markdownit().use(amsmath, { renderer })

This simplifies the interface to the package, and the options that we may decide to pass into amsmath are actually options of amsmath, that may change in the future.