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

modify currency regexes to handle decimal points #40

Merged
merged 1 commit into from
Aug 31, 2014
Merged

modify currency regexes to handle decimal points #40

merged 1 commit into from
Aug 31, 2014

Conversation

jhvst
Copy link
Collaborator

@jhvst jhvst commented Aug 28, 2014

For original issue ticket see: #39

I decided to leave out the zero omission, as I couldn't come up with any programming languages which would output floats as so. There was also a typo in comments on line 117, which I fixed. I also added all current currencies to the suffixed regex, even though only euros are actually typed so. We in Europe tend to many times forget that US dollars are prefixed with the sign.

Since there were no tests, I used http://regex101.com to come up with inputs of my own. So far, I haven't come up with any inputs which would result in bad data, although the third regex (on line 118) parses numbers such as 111.111.111 as valid. I don't really see a case where this would happen, but in case you see this as something you want sorted out, I could take a look at it. I don't just see it to affect this particular PR, so I'll let it untouched for now.

Also, grunt updated the version into 1.6.4, but seemingly the package.json already has that, so I guess you know what is going on. That being said, the primary Github branch is also called gh-pages.

tristen added a commit that referenced this pull request Aug 31, 2014
modify currency regexes to handle decimal points
@tristen tristen merged commit 45e95e1 into tristen:gh-pages Aug 31, 2014
@tristen
Copy link
Owner

tristen commented Aug 31, 2014

👍

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.

2 participants