-
-
Notifications
You must be signed in to change notification settings - Fork 57
Make GenerateConfusables generate NFKD instead of NFKC #1205
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
Conversation
Also fix a typo in the name of the method This fixes unicode-org/properties#459
This is ready for review so please take a look. (I didn't |
I'll look at it next week. (I'd replied to a github email, but that seems to have bounced.) |
@macchiati: We verified that some of the output changes are correct/desirable, such as the change from caron to breve for U+01C6. Mark please review. |
Ping @macchiati |
@roozbehp FYI -- I rephrased the PR description from simple "This fixes (issue)" to prevent GitHub from auto-closing the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can kind-of-rubber-stamp but have an optional code improvement.
unicodetools/src/main/java/org/unicode/text/UCD/GenerateConfusables.java
Outdated
Show resolved
Hide resolved
PTAL. |
@eggrobin Why does GitHub think I'm changing the repertoire?! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can ignore the CI check failures here.
private static Normalizer INSTANCE; | ||
static String normalize(String cf) { | ||
if (INSTANCE == null) { | ||
INSTANCE = new Normalizer(UCD_Types.NFKD, Default.ucdVersion()); | ||
INSTANCE.setSpacingSubstitute(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work to have the constructor unconditionally create the instance...
I probably misled you with suggesting normalize()
as a static
method.
Anyway, nicer than before, and we can revisit later.
Also fix a typo in the name of the method
This fixes problems described in the following issue.
Rephrased from simple "This fixes (issue)" to prevent GitHub from auto-closing the issue.