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

Parsing thousand separators #112

Closed
Doerge opened this issue Nov 10, 2019 · 3 comments
Closed

Parsing thousand separators #112

Doerge opened this issue Nov 10, 2019 · 3 comments

Comments

@Doerge
Copy link

Doerge commented Nov 10, 2019

Awesome with a completely new parsing implementation! πŸš€

I found a small bug: Some locales have reversed the thousand and the decimal separators.
Old version (before new parser):

iex(1)> Money.parse("1,127.54 €", locale: "en")
#Money<:EUR, 1127.54>
iex(2)> Money.parse("1.127,54 €", locale: "de")
#Money<:EUR, 1127.54>

new version:

iex(1)> Money.parse("1,127.54 €", locale: "en")
#Money<:EUR, 1127.54>
iex(2)> Money.parse("1.127,54 €", locale: "de")
{:error, {Money.ParseError, "Could not parse \"1.127,54 €\"."}}

Additionally I think this should throw an error:

iex(1)> Money.parse("1.127,54 €", locale: "en")
#Money<:EUR, 1.12754>

because what is the semantic meaning of the ,? It's clearly an error, either in input, or in locale choice.

@kipcole9
Copy link
Owner

Thanks for the comments and support. The first issue is a parsing error which I'll look into today. Once parsed correctly there should be no issues in understanding the number itself in the de locale.

I don't believe the second part is a bug per se. CLDR does permit grouping symbols in the fractional part:

The grouping separator may also occur in the fractional part, such as in β€œ#,##0.###,#”. This is most commonly done where the grouping separator character is a thin, non-breaking space (U+202F), such as β€œ1.618β€―033β€―988β€―75”. See physics.nist.gov/cuu/Units/checklist.html.

@kipcole9
Copy link
Owner

Fixed in master, thanks much for the report. I will push a new release to hex by end of Sunday.

@Doerge
Copy link
Author

Doerge commented Nov 10, 2019

Awesome, thanks!
Huh. Guess I learned something new about fractional thousand separators! 😁

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

No branches or pull requests

2 participants