search

validatorjs / validator.js

String validation
MIT License
23.12k stars 2.31k forks source link

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

Open OlllllllO opened 6 years ago

OlllllllO commented 6 years ago

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

Maxim-Mazurok commented 5 years ago

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

profnandaa commented 5 years ago

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

OlllllllO commented 3 years ago

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

Maxim-Mazurok commented 3 years ago

IDK why this PR wasn't merged: https://github.com/validatorjs/validator.js/pull/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 commented 3 years ago

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

sanchit36 commented 2 years ago

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 commented 2 years ago

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