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

Added en-IN localization #655

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

theY2Kbug
Copy link

@theY2Kbug theY2Kbug commented Jul 14, 2022

One single commit with all changes. Please refer to #650 for more details.

@theY2Kbug
Copy link
Author

theY2Kbug commented Jul 16, 2022

@BenjaminVanRyseghem
Refer to Why does "npm install" rewrite package-lock.json? for reasons to switch to npm ci instead.

@theY2Kbug
Copy link
Author

@BenjaminVanRyseghem

I'm going to stop posting updates here. Unless I get feedback, I cannot improve my contribution.

One final request, please do pull my changes and let me know if you are able to replicate the error I mentioned in #650 locally in your setup. After spending quite some time to fix this bug, I have a strong feeling that its a local bug (something to do with my setup).

@theY2Kbug
Copy link
Author

@BenjaminVanRyseghem

Awaiting workflow approval, any chance of merging this?

@IsotoxalDev
Copy link

I really want en-IN merged! why is there no response from the maintainer yet?

@BenjaminVanRyseghem
Copy link
Owner

I'm in the process of reviewing this code.
There are several points I would like to address, and I'm still thinking about how to do it:

  • I'm not very fond of having code that is this specific to a language in the core of the lib, with indian names on top of it;
  • there is a hard to track bug that is not fixed

I'm trying to find solutions for thoses 2 points, but it's complex, and I'm slightly running out of time right now (vacations and work deadlines)

@theY2Kbug
Copy link
Author

theY2Kbug commented Aug 17, 2022

  • I totally agree with you, having language specific conditional checks in the core of the library is not the ideal way to approach this problem, which is why if you notice line 374 of src/formatting.js, I've explicitly mentioned that the conditional check is subject to change in the future. Also, at present there are four countries that officially use the Indian Number System (India, Pakistan, Myanmar and Bangladesh) and other countries like Sri Lanka, Bhutan, to name a few use "lakh" and "crore" instead of the international system (million, billion, etc). So the bigger picture here is we could create a family of countries/currencies that use the Indian style denomination, implement all of them at once, and check which family a selected country/currency belongs to and trigger specific code that formats accordingly
  • Now coming to the error, Were you able to replicate it locally in your setup? It seems very peculiar that its untraceable

I've spent a lot of time on this, and I don't want the effort I put in to go wasted. If there's any suggestions or clarifications, please feel free to hit me up, I'm definitely willing to help integrate support for en-IN.

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

3 participants