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

Add support for culture #167

Closed
wants to merge 18 commits into from
Closed

Conversation

cristiingineru
Copy link

There are multiple types of numerals, even the arabic numberals have multiple representations: https://en.wikipedia.org/wiki/Eastern_Arabic_numerals#Numerals

This change is adding support for different type of "cultures" by taking advantage of the standard Intl.NumberFormat

The language itself is not enough to format the numerals because there are languages which support multiple formats.

Questions:

  1. Should both the arguments of the Intl.NumberFormat constructor - locales and options - be further exposed through the js-lingui API?
  2. Should the locales be further exposed as locales or culture? Other languages are using culture, or cultureNames...
  3. Should the information regarding the culture (or locales) be passed as arguments as in this PR or stored in i18n?

@cristiingineru
Copy link
Author

The tests are passing on my machine, probably the runtime is different on the CI machines.

One failure is related to the german culture (de-DE) which doesn't seams to be supported and the numerals are formatted using the english (en-UK) version.

The second one is even more interesting... string.replace doesn't work as expected when the strings are fully or partially containing arabic characters. At least this one I can reproduce it locally: it's working in a way in browser and in a different way in node.

I'm investigating both issues, but suggestions are welcome!

@tricoder42
Copy link
Contributor

Hi @cristiingineru,
thanks for PR! I'll take a look this weekend.

@cristiingineru
Copy link
Author

Hi @tricoder42!
The last commit inserted some code for investigating the failures on AppVeyor and CircleCI. Apparently full-icu didn't installed correctly or node is not using it (hasFullICU: false) and the formatting numerals function falls back to the default of the runtime, english, which explain both failures.

Do you have any time limits on any of the CI services? Each debug attempt to try something new will trigger other builds and I don't want to cause any issues. I noticed CircleCI has debug capabilities with SSH, so I'll try to debug the issue on one of my dummy repos which is also connected to CircleCI, but I never used SSH with this service yet and not sure I'll be able to reproduce the same condition as of this repo.

@cristiingineru
Copy link
Author

I've messed up this branch so badly, but I've figured it out how to set up the full-icu correctly and make the test pass. I'm going to close this PR and open a new one. Sorry for this mess!

@tricoder42
Copy link
Contributor

No problem at all. You could simply squash commits in your branch and force-push it.

@tricoder42
Copy link
Contributor

Sorry I wasn't any help in last days :/ I'm available in next 2 hours, ping me if you need anything. Looking forward to your PR!

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