Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Support \let via macros option #3738

Merged
merged 4 commits into from Apr 17, 2023
Merged

fix: Support \let via macros option #3738

merged 4 commits into from Apr 17, 2023

Conversation

edemaine
Copy link
Member

What is the previous behavior before this PR?
As described in #3737, katex.renderToString("\\int", {macros: {"\\Oldint":{"tokens":[{"text":"\\int","noexpand":true}],"numArgs":0,"unexpandable":false},"\\int":{"tokens":[{"text":"\\limits"},{"text":"\\Oldint"}],"numArgs":0,"delimiters":[[]]}}}) results in an infinite loop.

What is the new behavior after this PR?
Fixes #3737

The issue turned out to be how we handled the return value of expandOnce. We assumed that, if the return value isn't an Array, it's an instanceof Token. This isn't necessary true with a user's macros object, and given that we don't currently export Token, it's pretty difficult to bypass.

Given that we never actually use the array return values from expandOnce, I changed the return value for expandOnce to either a number (to indicate the number of expanded tokens, so you could still look up the tokens in the stack if you wanted to) or false (to indicate no expansion happened). We can't use 0 for the latter because an actual expansion might result in zero tokens. The resulting code is arguably cleaner.

I also documented that macros can have object expansions, and specified how to simulate \let.

Issue #3737 turned out to be how we handled the return value of `expandOnce`.
We assumed that, if the return value isn't an `Array`, it's an
`instanceof Token`.  This isn't necessary true with a user's `macros`
object, and given that we don't currently export `Token`, it's pretty
difficult to bypass.

Given that we never actually use the array return values from
`expandOnce`, I changed the return value for `expandOnce` to either a
`number` (to indicate the number of expanded tokens, so you could still
look up the tokens in the stack if you wanted to) or `false`
(to indicate no expansion happened).  We can't use `0` for the latter
because an actual expansion might result in zero tokens.
The resulting code is arguably cleaner.

I also documented that `macros` can have object expansions, and
specified how to simulate `\let`.

Fixes #3737
@codecov
Copy link

codecov bot commented Oct 22, 2022

Codecov Report

Merging #3738 (499f178) into main (c7b1f84) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 499f178 differs from pull request most recent head 6be0c2b. Consider uploading reports for the commit 6be0c2b to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3738   +/-   ##
=======================================
  Coverage   92.98%   92.98%           
=======================================
  Files          91       91           
  Lines        6770     6770           
  Branches     1574     1574           
=======================================
  Hits         6295     6295           
  Misses        437      437           
  Partials       38       38           
Impacted Files Coverage Δ
src/defineMacro.js 100.00% <ø> (ø)
src/MacroExpander.js 97.43% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

@ronkok
Copy link
Collaborator

ronkok commented Oct 28, 2022

Would you be willing to consider an alternate approach? In the Temml library, I have exposed an additional function, definePreamble(<str>). A web site administrator would run this function before any math is rendered, as in this example:

// Optional preamble.
const macros = temml.definePreamble(
    `\\let\\Oldint=\\int
    \\def\\int{\\Oldint\\limits}
    \\newcommand\\d[0]{\\operatorname{d}\\!}
    \\def\\foo{x^2}`
);

As you can see, the argument is a string containing macros written as they would be written in a document. The function returns a macros object. This approach gives an author a way to write macros in a syntax that they already know. No need to learn how the macros object is internally constructed.

definePreamble() was not that hard to code. In the Temml.js source file, I added and exposed a new function:

/**
 * Take an expression which contains a preamble.
 * Parse it and return the macros.
 */
const definePreamble = function(expression, options) {
  const settings = new Settings(options);
  settings.macros = {};
  if (!(typeof expression === "string" || expression instanceof String)) {
    throw new TypeError("Temml can only parse string typed expression")
  }
  const parser = new Parser(expression, settings, true)
  // Blank out any \df@tag to avoid spurious "Duplicate \tag" errors
  delete parser.gullet.macros.current["\\df@tag"]
  const macros = parser.parse()
  return macros
};

As you can see, the function uses the existing parser to do the work. No need to reinvent a new wheel. If you look very closely, you can see that the call to new Parser() includes a new, third, optional argument. It’s a boolean named isPreamble.

After the parser has parsed the top-level expression, it checks if it is engaged in a preamble. If so, it extracts the macros and executes a quick return.

if (this.isPreamble) {
    const macros = Object.create(null)
    Object.entries(this.gullet.macros.current).forEach(([key, value]) => {
        macros[key] = value
      })
    this.gullet.endGroup();
    return macros
}

That’s about it. Temml does not have a globalGroup rendering option, which may complicate a KaTeX version of definePreamble. Hopefully not too much.

@edemaine
Copy link
Member Author

@ronkok KaTeX already has the equivalent of definePreamble (though uglier):

const macros = {}
katex.renderToString(`...\let\foo=bar...`, {macros, globalGroup: true})

(The main motivation for globalGroup, I believe, is running in the preamble.)

It may be nice to add a helper for this, but at least it's possible. Probably we should document this approach at least.

I still think this PR is an improvement over the current code, making the macros object a bit more robust. (It's currently very easy to cause KaTeX to crash by passing in bad macros.) But perhaps I should remove the documentation of how to do \let this way, and instead add documentation of the above approach. What do you think?

Copy link
Collaborator

@ronkok ronkok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the code changes look good. I've proposed some re-wording of the docs for macros.

docs/options.md Outdated
- Each macro is a property with a name like `\name` (written `"\\name"` in JavaScript) which maps to a string that describes the expansion of the macro, or a function that accepts an instance of `MacroExpander` as first argument and returns the expansion as a string. `MacroExpander` is an internal API and subject to non-backwards compatible changes. See [`src/defineMacro.js`](https://github.com/KaTeX/KaTeX/blob/main/src/defineMacro.js) for its usage.
- Single-character keys can also be included in which case the character will be redefined as the given macro (similar to TeX active characters).
- *This object will be modified* if the LaTeX code defines its own macros via `\gdef` or `\global\let` (or via `\def` or `\newcommand` or `\let` when using `globalGroup`), which enables consecutive calls to KaTeX to share state.
- Expansions can also be objects matching [an internal `MacroExpansion` specification](https://github.com/KaTeX/KaTeX/blob/main/src/defineMacro.js), which is what results from global `\def` or `\let`. For example, you can simulate the effect of `\let\realint=\int` via `{"\\realint": {tokens: [{text: "\\int", noexpand: true}], numArgs: 0}}`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to propose an edited version of the macros entry, with an eye to toward less experienced webpage administrators.

  • macros: object. A collection of custom macros.

    • Each macro is a key-value pair in which the key is a new KaTeX function name and the value is the expansion of the macro. Example: macros: {"\\R": "\\mathbb{R}"}.

    • To be more precise, each macro is a property with a name like \name (written "\\name" in JavaScript) which maps to a string that describes the expansion of the macro, or a function that accepts an instance of MacroExpander as first argument and returns the expansion as a string. MacroExpander is an internal API and subject to non-backwards compatible changes. See src/defineMacro.js for its usage.

    • Single-character keys can also be included in which case the character will be redefined as the given macro (similar to TeX active characters).

    • The macros object will be modified if the LaTeX code defines its own macros via \gdef or \global\let (or via \def or \newcommand or \let when using globalGroup). This enables macro definitions to persist between calls to KaTeX.

    • Expansions can also be objects matching an internal MacroExpansion specification, which is what results from global \def or \let. For example, you can simulate the effect of \let\realint=\int via {"\\realint": {tokens: [{text: "\\int", noexpand: true}], numArgs: 0}}.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments, and sorry for taking so long to get to this. I just took a revision pass inspired by your feedback. Let me know if you have further comments, or I'll merge later today. (We can always edit further in another PR.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macros entry looks much better. This is good to go.

Copy link
Collaborator

@ronkok ronkok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good code. Let's go with it.

@edemaine edemaine enabled auto-merge (squash) April 17, 2023 19:55
@edemaine edemaine merged commit bdb0be2 into main Apr 17, 2023
8 checks passed
@edemaine edemaine deleted the macros-let branch April 17, 2023 19:59
KaTeX-bot added a commit that referenced this pull request Apr 17, 2023
## [0.16.6](v0.16.5...v0.16.6) (2023-04-17)

### Bug Fixes

* Support `\let` via `macros` option ([#3738](#3738)) ([bdb0be2](bdb0be2)), closes [#3737](#3737) [#3737](#3737)
@KaTeX-bot
Copy link
Member

🎉 This PR is included in version 0.16.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Equivalent of \let via macros options
3 participants