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

Default match broken for cyrillic/russian because of incorrect diactrics removal #57

Closed
glebtv opened this issue Apr 15, 2019 · 2 comments
Labels

Comments

@glebtv
Copy link
Contributor

glebtv commented Apr 15, 2019

I opened an issue with node-diactrics but it was not updated for 4 years, so opening an issue here too.
nijikokun/Diacritics.js#6

  • match-sorter version: 2.3.0
  • node version: v11.13.0
  • npm (or yarn) version: 1.15.2

Relevant code or config

console.log(diacritics.clean("л"))

л Л is a lower/upercase case cyrillic/russian L and should not be replaced with latin n when matching.

This breaks case-insensitive search in match-sorter with default settings for any cyrillic text which has this letter.

node-diacritics also has this problem:
andrewrk/node-diacritics#32

but remove-accents does not: https://github.com/tyxla/remove-accents

Example word which does not match: "Лед"

What you did:

const list = ["Лед", "лед"];
const filtered = matchSorter(list, "л");

What happened:

["лед"], both items should match

Reproduction repository:

https://codesandbox.io/s/r76w8oqo8o

Problem description:

Suggested solution:

  • Default keepDiacritics to true
  • Replace used library
  • allow user to bring his own diactrics library
@kentcdodds
Copy link
Owner

Hi @glebtv.

Thanks for the report. If you have a better diacritics library, please make a PR to swap with the new one and I'll give it a look.

Also, you can probably use your bundler's configuration to alias the diacritics module with a custom one which would be even better than "allow user to bring his own diactrics library" because it would allow you to avoid including the diacritics package in your bundle as well.

I'll look forward to a PR :)

@kentcdodds
Copy link
Owner

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants