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

Strip diacritics in Greek alphabet strings #660

Closed
wants to merge 2 commits into from
Closed

Strip diacritics in Greek alphabet strings #660

wants to merge 2 commits into from

Conversation

ianatha
Copy link

@ianatha ianatha commented Aug 11, 2021

What issue does this pull request resolve?
Fixes a portion of the problem for #659

What changes did you make?
Added rules in stripDiacritics for Modern Greek diacritics.

Is there anything that requires more attention while reviewing?
No.

@ericgio
Copy link
Owner

ericgio commented Sep 6, 2021

Hey @ianatha, thanks for opening issue #659 and for putting up a fix! For the test case you provided, it seems like using string.normalize is a better bet, since it doesn't require adding additional entries to the map. That would require dropping support for IE11, which would make it a breaking change. I think it's reasonable to drop support for IE, and is something I've been planning.

@ericgio
Copy link
Owner

ericgio commented Oct 11, 2021

Addressed in v6.0.0-alpha.1 using string.normalize.

@ericgio ericgio closed this Oct 11, 2021
@ianatha ianatha deleted the patch-1 branch November 27, 2021 12:02
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.

None yet

2 participants