Skip to content

Finishes TeX Error localisation#1479

Open
zorkow wants to merge 12 commits into
feature/localefrom
feature/locale_finish_tex
Open

Finishes TeX Error localisation#1479
zorkow wants to merge 12 commits into
feature/localefrom
feature/locale_finish_tex

Conversation

@zorkow
Copy link
Copy Markdown
Member

@zorkow zorkow commented May 19, 2026 •
edited
Loading

PR (nearly) finished the localisation of the tex module. In particular, it

  • removes all duplicates from the extension packages
  • puts all error messages into the right place
  • errors in BaseItem and subclasses now get the Component string and id.
  • introduces a texError function that throws the localised TexError. This way we do not have a breaking change. texError gets type never, which is needed to avoid typescript errors, but which makes it a bit awkward to test.

Finally, the only point still to be discusses is how to deal with the getPackage errors, in particular since there is also a warning. Should we make the Error a TexError and localise the warning?

Note, that I have added copy blocks to all the necessary config.json files in the components. Once PR #1476 hits develop I will merge that in and resolve the conflicts.

@zorkow zorkow requested a review from dpvc May 19, 2026 12:11
dpvc
dpvc approved these changes May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

This all looks good, with one recommended change.

Comment thread ts/util/Locale.ts Outdated
Base automatically changed from feature/locale_plus2 to feature/locale May 19, 2026 15:43
@dpvc dpvc mentioned this pull request May 19, 2026
Draft
@dpvc
Copy link
Copy Markdown
Member

dpvc commented May 19, 2026

Looks like this needs to have pnpm -s format:fix run on it.

dpvc
dpvc reviewed May 20, 2026
View reviewed changes
Comment thread ts/input/tex/physics/PhysicsItems.ts Outdated
@dpvc
Copy link
Copy Markdown
Member

dpvc commented May 20, 2026 •
edited
Loading

Does this line

MathJax-src/ts/input/tex/Tags.ts

Line 699 in fcf47d9

throw Error('Unknown tags class');

need to be localized?

And also this one?

MathJax-src/ts/input/tex/MapHandler.ts

Line 205 in fcf47d9

console.log('TexParser Warning: ' + message);

The first can be done through Locale.error(). Should there also be a Locale function for the second? Like Locale.log()? (Or should Locale.error() use console.error() and a new function Locale.throw() use throw?)

@dpvc
Copy link
Copy Markdown
Member

dpvc commented May 20, 2026

Here are a couple more:

MathJax-src/ts/input/tex/bussproofs/BussproofsUtil.ts

Lines 654 to 656 in fcf47d9

throw Error(
'The bussproofs extension requires an output jax with a getBBox() method'
);

MathJax-src/ts/input/tex/require/RequireConfiguration.ts

Line 215 in fcf47d9

throw Error('Illegal characters used in \\require prefix');

MathJax-src/ts/input/tex/Configuration.ts

Line 441 in fcf47d9

throw Error(`Package '${name}' doesn't target the proper parser`);

dpvc
dpvc requested changes May 20, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

There are problems with the jsdoc comments, here.

Comment thread ts/util/Locale.ts Outdated
@dpvc dpvc mentioned this pull request May 20, 2026
Open
@zorkow
Copy link
Copy Markdown
Member Author

zorkow commented May 21, 2026

The first can be done through Locale.error(). Should there also be a Locale function for the second? Like Locale.log()? (Or should Locale.error() use console.error() and a new function Locale.throw() use throw?)

There are a couple of issues here:

  1. Both MapHandler and Configuration have a warn method. Those should be removed.
  2. I feel an error function should always throw an error. Otherwise, why call it an error.
  3. We could have a general Locale.warn method that gives localised information on the console (via console.info).
  4. I don't think logging should be localised.

@zorkow
Copy link
Copy Markdown
Member Author

zorkow commented May 21, 2026

Does this line

MathJax-src/ts/input/tex/Tags.ts

Line 699 in fcf47d9

throw Error('Unknown tags class');

need to be localized?

This is now localised.

dpvc
dpvc approved these changes May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

OK, this is a go for now. I will handle the other throw commands in my PR that we discussed in our meeting.

@dpvc
Copy link
Copy Markdown
Member

dpvc commented May 21, 2026

Merging feature/mhchem-locale into feature/locale had introduced a merge conflict here. I can fix that, if you like.

@zorkow
Copy link
Copy Markdown
Member Author

zorkow commented May 22, 2026

Does this line

MathJax-src/ts/input/tex/Tags.ts

Line 699 in fcf47d9

throw Error('Unknown tags class');

need to be localized?

This is now localised.

This now has a localisation as UnknownTag but is not a TexError.

@zorkow
Copy link
Copy Markdown
Member Author

zorkow commented May 22, 2026

Review suggestions incorporated. I left the regular Error messages alone as discussed yesterday to do them in a separate PR.
PTAL

@zorkow zorkow requested a review from dpvc May 22, 2026 08:32
dpvc
dpvc approved these changes May 22, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

Looks good.

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

Reviewers

@dpvc dpvc dpvc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zorkow @dpvc