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 issue with \textit, etc., not working with textmacros extension. (mathjax/MathJax#2514) #546

Merged
merged 2 commits into from
Sep 12, 2020

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Sep 4, 2020

The textmacros extension resets the font to normal when \text{} is used (as it does in actual LaTeX), since you can change fonts internally within \text{} with textmacros. (This differs from \text{} without textmacros, since the only way to set the font within \text{} previously was to set the font outside it, a conscious decision to differ from LaTeX to allow that functionality.) But because \textit{} and the other similar macros were defined as macros that used {\it\text{#1}}, the change in behavior for \text{} caused these macros to fail with the textmacros extension.

This PR changes the implementations of \textit{} and the other similar macros to use javascript functions (rather than replacement macros) that get passed the needed mathvariant for initializing the font. The \text{} macro retains its original behavior without textmacros, but has its new behavior with the extension, and now all the related ones now work with the extension as well.

Resolves issue mathjax/MathJax#2514

@dpvc dpvc requested a review from zorkow September 4, 2020 18:53
@dpvc dpvc added this to the 3.1.1 milestone Sep 4, 2020
@dpvc dpvc changed the title Fix issue with \textit, etc., not working with textmacros extension (mathjax/MathJax#2514) Fix issue with \textit, etc., not working with textmacros extension. (mathjax/MathJax#2514) Sep 8, 2020
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.

That change makes the implementation of the \textXX macros a lot cleaner.
Just one trivial change.

@@ -1003,10 +1003,11 @@ BaseMethods.BuildRel = function(parser: TexParser, name: string) {
* @param {TexParser} parser The calling parser.
* @param {string} name The macro name.
* @param {string} style Box style.
* @param {srting} font the mathvariant to use
Copy link
Member

@zorkow zorkow Sep 11, 2020

Choose a reason for hiding this comment

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

Capitalise. And typo.

@zorkow
Copy link
Member

zorkow commented Sep 11, 2020

I think once 3.1.1. is out I should spend some time to extend our tests. In particular

  1. Systematically add test cases for every single macro that we have. While most extension packages have tests for every macro, for the ones in Base and AMS I primarily constructed tests for all the cases in the original v2 methods only.

  2. Run tests multiple time, adding other extensions successively so we can detect unwanted interactions automatically in the future.

  3. Automate all that via some CI tool.

@dpvc dpvc merged commit 8eda1e3 into develop Sep 12, 2020
@dpvc dpvc deleted the issue2514 branch September 12, 2020 13:53
@dpvc
Copy link
Member Author

dpvc commented Sep 12, 2020

I think once 3.1.1. is out I should spend some time to extend our tests.

That sounds good.

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