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

Feature Request: Currency: Option to Require Thousands Separator #912

Open
OlllllllO opened this issue Oct 15, 2018 · 7 comments
Open

Feature Request: Currency: Option to Require Thousands Separator #912

OlllllllO opened this issue Oct 15, 2018 · 7 comments

Comments

@OlllllllO
Copy link

We're using this to help validate data that includes currency. We want to make sure that a comma separator is always present.

@OlllllllO OlllllllO changed the title FEATURE REQUEST: Currency: Option to Require Thousand Separator Feature Request: Currency: Option to Require Thousand Separator Oct 15, 2018
@OlllllllO OlllllllO changed the title Feature Request: Currency: Option to Require Thousand Separator Feature Request: Currency: Option to Require Thousands Separator Oct 15, 2018
@Maxim-Mazurok
Copy link
Contributor

There's an pull request open to resolve that issue: #945

@profnandaa
Copy link
Member

Thanks @Maxim-Mazurok, will review and get back 👍

ezkemboi added a commit to ezkemboi/validator.js that referenced this issue Aug 11, 2019
@OlllllllO
Copy link
Author

@profnandaa - hello. any update on when this feature will get merged in? thanks.

@Maxim-Mazurok
Copy link
Contributor

Maxim-Mazurok commented Dec 5, 2020

IDK why this PR wasn't merged: #1084
It looks like a totally valid approach to me. There's no such thing as too many options, especially when people asked for this option... IMO

@OlllllllO
Copy link
Author

#1084 was closed without getting merged and it seems that this feature just got lost in the mix. Any chance of revisiting?

@sanchit36
Copy link

Hey, i am looking for my first contribution in open source. Can anyone guide me with this issue i would i like to work on it

@WikiRik
Copy link
Member

WikiRik commented Jan 14, 2022

Hey, i am looking for my first contribution in open source. Can anyone guide me with this issue i would i like to work on it

#1084 solved this issue already, but the choice was made by the maintainers to reject that approach in search for a larger refactor of isCurrency which would limit the amount of options that is possible.
One option that would involve this thousands separator is to merge the allow_ and require_ options so it's not a Boolean but allows a few options. My idea would be something like; required, allowed/optional, forbidden. But feel free to suggest a different wording. This way we can reduce the total amount of options slightly and add this requested functionality.
For implementation you can check out the above-mentioned PR

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

Successfully merging a pull request may close this issue.

7 participants