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

More validators #684

Merged
merged 6 commits into from
Aug 22, 2017
Merged

More validators #684

merged 6 commits into from
Aug 22, 2017

Conversation

0xch4z
Copy link
Contributor

@0xch4z 0xch4z commented Jul 18, 2017

Everything is up to spec. Referenced Unicode CLDR for postal code standards. I made a huge simplification of the recommend Great Britain Regex as it was too big for a lightweight library like this, but it works just fine.

Original GB regex (not js regex engine):

GIR[ ]?0AA|((AB|AL|B|BA|BB|BD|BH|BL|BN|BR|BS|BT|CA|CB|CF|CH|CM|CO|CR|CT|CV|CW|DA|DD|DE|DG|DH|DL|DN|DT|DY|E|EC|EH|EN|EX|FK|FY|G|GL|GY|GU|HA|HD|HG|HP|HR|HS|HU|HX|IG|IM|IP|IV|JE|KA|KT|KW|KY|L|LA|LD|LE|LL|LN|LS|LU|M|ME|MK|ML|N|NE|NG|NN|NP|NR|NW|OL|OX|PA|PE|PH|PL|PO|PR|RG|RH|RM|S|SA|SE|SG|SK|SL|SM|SN|SO|SP|SR|SS|ST|SW|SY|TA|TD|TF|TN|TQ|TR|TS|TW|UB|W|WA|WC|WD|WF|WN|WR|WS|WV|YO|ZE)(\d[\dA-Z]?[ ]?\d[ABD-HJLN-UW-Z]{2}))|BFPO[ ]?\d{1,4}

Let me know if I need to change anything!

Thanks,
Charles

@landsman
Copy link

great job!

validator.js Outdated




Copy link
Member

Choose a reason for hiding this comment

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

Could the empty lines be removed?

@0xch4z
Copy link
Contributor Author

0xch4z commented Jul 21, 2017

Oops... I suppose that whitespace was randomly added in the build step. Fixed.

@chriso
Copy link
Collaborator

chriso commented Aug 22, 2017

Thanks for the PR 😄

return patterns[locale].test(str);
} else if (locale === 'any') {
// Test each unique pattern
return !![...new Set(Object.values(patterns))].find(pattern => pattern.test(str));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you stick with ES5 here? It looks like some newer features are preserved in validator(.min).js and are causing compat issues with older Node versions (e.g. #681).

@chriso
Copy link
Collaborator

chriso commented Aug 22, 2017

Perfect, thanks.

@chriso chriso merged commit a9327d5 into validatorjs:master Aug 22, 2017
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var lat = /^\(?[+-]?(90(\.0+)?|[1-8]?\d(\.\d+)?)$/;
var long = /^\s?[+-]?(180(\.0+)?|1[0-7]\d(\.\d+)?|\d{1,2}(\.\d+)?)\)?$/;

Choose a reason for hiding this comment

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

Why is an optional whitespace tolerated at the beginning of long, but not at the end of long or the beginning or end of lat?

Copy link
Contributor Author

@0xch4z 0xch4z Aug 22, 2017

Choose a reason for hiding this comment

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

Because it is pretty common to separate coordinates with a comma and a space, but the space should not be mandatory. It's not common however, to prefix with a space, or end with a space. That's needless and would look incredibly silly.

@markstos
Copy link

@Charliekenney23 I had in mind the [Robustness Principle]( be liberal in what you accept from others) "be liberal in what you accept from others"

While I don't recommend creating coordinate strings with leading and trailing space or with a space before the comma, I very well expect that I might run into such data that has been hand-typed in a spreadsheet by a non-technical user that gets uploaded to my server.

It seems like a simple addition to make the library more robust. Even if these extra spaces are present, there's no doubt that the string would be intended to a be coordinate string.

As I understand, the goal is not to parse a "coordinate string standard", but rather to validate input "in the wild" that may be provided by humans or other computers that is expected to some amount of tolerable imperfections.

@0xch4z
Copy link
Contributor Author

0xch4z commented Aug 22, 2017

@markstos I totally understand what you're getting at here. Though, the same could be said for every validator in this repository that evaluates single line input data (e.g. email, phone number, ip address; which are also not preceded or appended with a none-or-more whitespace match). The norm would be to simply call the trim method on the string of subject before you even evaluate the string, then you know your data is normalized from the start.

As this library aims to provide convenience, it would make sense to side with the norm of strict validation. Beyond that, it seems sloppy and redundant to add '\s*' to the start and end of every expression in this library. This should be left up to the user IMO.

edit: grammar

@markstos
Copy link

@Charliekenney23 I get what you saying about not re-inventing trim().

I still think it would be helpful to tolerate the "space before comma" case, which is something that comes up from time-to-time in hand-entered CSV data.

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.

Postcode Feature Request - isLatLong(lat-long-pair)
5 participants