Skip to content

Properly handle comments in arguments and other places, and spaces in macro templates. (mathjax/MathJax#3577) - #1539

Open
dpvc wants to merge 1 commit into
developfrom
fix/comments
Open

Properly handle comments in arguments and other places, and spaces in macro templates. (mathjax/MathJax#3577)#1539
dpvc wants to merge 1 commit into
developfrom
fix/comments

Conversation

@dpvc

@dpvc dpvc commented Aug 5, 2026

Copy link
Copy Markdown
Member

This PR brings MathJax's handing of percent-sign comments for macros more in line with actual LaTeX, both in terms of calling a macro and defining one. In the past, percent signs could be part of a template macro's delimiter strings, and multiple spaces counted as significant in the template, but in actual LaTeX, percent signs act as comments within the template (the content is not part of the template) and multiple spaces are collapsed. Also, when calling a macro, percent signs were not properly acting as comments within the arguments.

These issues are resolved here, but since this could be a breaking change to existing content, two new options are added, legacyComments and legacyTemplateMacros to control whether the new processing or the legacy processing is to be performed.

Resolves issue mathjax/MathJax#3577.


Details

A number of tests needed to be added to cover the new functionality, and some old tests were updated that operate differently under the new comment processing. In ConfigMacros.test.ts the runMacroTests() method doesn't need to run two tests, since the results are compared to snapshots rather than to each other, as they were in the past. That means the control test doesn't actually have to be performed, so it (and its parameter) is removed.

The new options are added in the input/tex.ts file.

In input/tex/NodeUtil.ts, setAttribute() and setProperty() now use the more-appropriate Property type rather than Args, since that's what the node's property object uses, and because Args needs to be more general in order to handle the new approach to template macros.

Much of the work for percent signs is done in input/tex/TexParser.ts. The GetNext() macro now handles removing comments, so that everything that uses GetNext() will properly handle comments. (We then adjust other functions that used to walk the TeX string by hand to now use GetNext().) A new clipComment() function is used to remove a comment from the input string. A new unescapePercents() is defined for use in handling arguments that are not further processed as TeX, like the contents of \mmlToken commands, so you can do \mmlToken{mo}{\%} to get <mo>%</mo>, or \mmlToken{text}{\\\%} to get <mtext>\%</mtext>.

The GetNext(), GetArgument(), and GetBrackets() functions take a new boolean argument that determines if the new percent-processing rules are to be used. (This allows the legacyComments option to get the old behavior.)

The extra return values are to avoid a warning that the function doesn't always have a return value. I don't like it, but I don't think it hurts.

In input/tex/Types.ts, the Args type is extended to allow string[] values. These are needed for template macros, which now use an array of tokens to look for rather than a fixed string. That allows multiple spaces to be collapsed, and comments to be removed from both the template itself, and the usage within a macro call.

In input/tex/base/BaseItems.ts, the getEntry() macro, which scans ahead for the end of the entry, now skips comments properly. (This isn't currently controlled by legacyComments, but I suppose it should be.)

In input/tex/base/BaseMethods.ts, the \mmlToken argument handles escaped percents. The loop in the Entry() method is modified to use GetNext() in order to handle comments. That means that we have to switch the roles of i and parser.i, since GetNext() uses parser.i.

The input/tex/bbox/BBoxConfiguration.ts file handles escaped percents in its argument (because it allows CSS specifier that might need to be percentages).

The loop in input/tex/cases/CasesConfiguration.ts is modified to use GetNext() as above, again requiring that i and parser.i be swapped.

In input/tex/configmacros/ConfigMacrosConfiguration.ts, if a macro is a templated one (def[2] is an array), we tokenize the elements in the array, since that is how the macro templates are now stored.

In input/tex/html/HtmlMethods.ts, the arguments are now processed for escaped percents, other than the CSS id, which is take verbatim. (I'm not sure why I did that, but it probably should be unescaped as well. Perhaps I just missed it when I added the service routine.)

In input/tex/newcommand/NewcommandMethods.ts, I replace a x ? y() : z() construct with an if-then. The other change is to use template token arrays rather than template strings. This uses the updates getParameter() call rather than the (removed) MatchParam() method (see NewcommandUtil.ts below).

The main changes are in input/tex/newcommand/NewcommandMethods.ts. Here we add methods for tokenizing a string that acts as one of the template separators. The tokens newlines converted to space, multiple spaces collapsed to one, and only a single (actual) space at the end.

Uses of this were changes to NewcommandUtil for consistency.

The GetTemplate() method is modified to return the parameter separators as token arrays instead of a single string. The loop is reorganized a bit to accommodate that.

The GetParameter() method is likewise modified in order to accept arrays of tokens instead of strings, and to look through the input string by tokens rather than looking for the exact string. That lets it handle comments and multiple spaces.

The SetOptions method in input/tex/setoptions/SetOptionsConfiguration.ts handles escaped percent signs, since the option may need a percentage (e.g., for multline widths).

Finally, the loop in input/tex/verb/VerbConfiguration.ts is modified to use GetNext(false). This allows you to do \verb|%| to get a monospaced percent sign, but note that you can't do this inside a macro definition, or within an argument that is passed to a macro, as the percent would eb traded as a comment in those cases. This is consistent with actual LaTeX.

@dpvc
dpvc requested a review from zorkow August 5, 2026 16:34
@dpvc dpvc added this to the v4.2 milestone Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.93%. Comparing base (a8088bb) to head (90f6b6a).

Files with missing lines Patch % Lines
ts/input/tex/TexParser.ts 94.28% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop    #1539     +/-   ##
==========================================
  Coverage    86.93%   86.93%             
==========================================
  Files          388      388             
  Lines        87571    87663     +92     
  Branches      3290     4984   +1694     
==========================================
+ Hits         76129    76210     +81     
+ Misses       11442    11432     -10     
- Partials         0       21     +21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant