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 to the currency for thousand delimiter when unknown #327

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

elfassy
Copy link
Contributor

@elfassy elfassy commented Aug 15, 2024

Why

The fuzzy parsing tries to guess the amount from a string. We can get a little more strict about the guesses we're making

What

When we're not able to guess if the delimiter is for thousands or decimals, defer to the currency decimal delimiter

when we're not able to make an educated guess on decimal vs thousand separator, default to using the decimal separator for the currency
Copy link
Contributor

@TayKangSheng TayKangSheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres a whole lot of commits in this PR. If i'm understanding correctly, the fix is in lib/money/core_extensions.rb where we call Money.new instead of running it through Parser::Fuzzy? 👍 Good on that change.

@elfassy
Copy link
Contributor Author

elfassy commented Aug 16, 2024

sorry the rebase on v3 caused a bit of noise on this PR, please have another look 🙏

@elfassy elfassy merged commit 18b9fe4 into v3 Aug 19, 2024
9 checks passed
@elfassy elfassy mentioned this pull request Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants