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

Make over/under braces and matrices be full size, as in actual TeX (mathjax/MathJax#3300) #1146

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Oct 24, 2024

This PR fixes some issues with the size of the contents of \overbrace, \underbrace, and matrices so that they match the output of actual LaTeX. The argument to the over and under brace macros is actually typeset in display style in LaTeX, and the matrices are in text mode.

The changes to MmlNode.ts fix a problem with the inherited values of displaystyle and scriptlevel so that they don't produce _inherit_ for example, which is supposed to be translated into the actual inherited value. This allows the new filter discussed below to work.

The smallmatrix and subarray environments used to use a scriptlevel property to tell mtable to use a different script level, but that is now handled by wrapping the tables with mstyle elements that set scriptlevel explicitly, and the property is changed to smallmatrix so, which affects the interline spacing, and needs to be preserved through a data-mjx-smallmatrix attribute. A similar mstyle is added for the brace macros.

I hate to see extra mstyle tags that are unnecessary, however, so I've also added another post-filter to the TeX input jax that removes unneeded ones (when the scriptlevel and displaystyle match the surrounding values). This also removes unneeded mstyle nodes from the spacing macros like \,, but keeps them when they are needed.

If you don't like the filter, that can be removed, and the mstyle nodes can be kept.

Resolves issue mathjax/MathJax#3300.

@dpvc dpvc requested a review from zorkow October 24, 2024 21:48
@dpvc dpvc added this to the v4.0 milestone Oct 24, 2024
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

lgtm.
But see my comment on BaseMethods.ts for consideration. Going forward, should we go for more concise code or for testable paths?

@@ -1961,6 +1961,8 @@ const BaseMethods: { [key: string]: ParseMethod } = {
if (style === 'S') {
// @test Subarray, Small Matrix
array.arraydef['scriptlevel'] = 1;
} else {
array.arraydef['scriptlevel'] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Could be done with a ternary expression of the form:

Suggested change
array.arraydef['scriptlevel'] = 0;
array.arraydef['scriptlevel'] = style === 'S' ? 1 : 0;

However, this way we get two test paths for completeness.

@dpvc
Copy link
Member Author

dpvc commented Nov 12, 2024

should we go for more concise code or for testable paths?

I'm for more concise code, to keep the bundle size down. While the coverage information is useful, it really doesn't mean that all the needed cases are actually covered, so I still think we need to look carefully at the code to produce a complete set of tests, so I would say we would pick this up then.

@dpvc dpvc merged commit 7f08013 into develop Nov 12, 2024
@dpvc dpvc deleted the issue3300 branch November 12, 2024 15:07
dpvc added a commit that referenced this pull request Nov 24, 2024
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.

2 participants