Fix handling of nonmath hyphens in mathtext. #23270
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anntzer
added
Release critical
hyphen-to-unicode-minus replacement now occurs at the parsing stage, and thus does not need to be done again in get_unicode_index (otherwise, text hyphens also get incorrectly substituted. Strictly speaking, one would need either to fully deprecate get_unicode_index and replace it by a private helper (for which `math` is always False), or introduce a deprecation period during which users *must* pass the `math` kwarg (any value other than False results in a warning) and then another one that deprecates that kwarg; which is quite long; hopefully the change is in a private-enough part of the library that we can just do it as is. Also do some more changes to the mathtext-svg-text-only-tests that I missed when the feature first went in: only test svg (that's the point of it...) and only test two fonts: a "standard" truetype font -- the default dejavusans -- and the "cm" font, which has a very specific encoding (see the BakomaFonts class).
oscargus
reviewed
Jun 14, 2022
| @@ -119,6 +119,7 @@ | |||
| # 'svgastext' tests switch svg output to embed text as text (rather than as | |||
| # paths). | |||
| svgastext_math_tests = [ | |||
| r'$-$-', | |||
oscargus
approved these changes
Jun 14, 2022
|
What about creating a helper function in |
|
If we are really bothered with the change here I would rather just make get_unicode_index fully private rather than keep reexporting it to a public module, but that seems a bit of a pity (in my view, the main useful third-party use of get_unicode_index is to map |
I agree. Was more thinking of the "breaking" change without deprecation. But then I have no idea how much of a problem that is. |
jklymak
approved these changes
Jun 22, 2022
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hyphen-to-unicode-minus replacement now occurs at the parsing stage, and
thus does not need to be done again in get_unicode_index (otherwise,
text hyphens also get incorrectly substituted.
Strictly speaking, one would need either to fully deprecate
get_unicode_index and replace it by a private helper (for which
mathis always False), or introduce a deprecation period during which users
must pass the
mathkwarg (any value other than False results in awarning) and then another one that deprecates that kwarg; which is quite
long; hopefully the change is in a private-enough part of the library
that we can just do it as is.
Also do some more changes to the mathtext-svg-text-only-tests that I
missed when the feature first went in: only test svg (that's the
point of it...) and only test two fonts: a "standard" truetype font --
the default dejavusans -- and the "cm" font, which has a very specific
encoding (see the BakomaFonts class).
Closes #23268, sorry about that :)
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).