-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(isISO6391): add ISO 639-1 validator #1892
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1892 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 103 +1
Lines 2072 2078 +6
Branches 472 472
=========================================
+ Hits 2072 2078 +6
Continue to review full report at Codecov.
|
I understand the desire to stick to regex based validation, but ISO will never update this spec with breaking changes, as far as I understand it. This specific version of ISO 639 is not likely to be updated ever again. It was last updated in 2002 and has since been reviewed three times without changes. |
I cleaned up the list of language codes so it's easier on the eyes, like the list in the ISO3166 validator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I expect limited changes (and if they occur only in additions) and doing regex based validation is not realistic for this one. Looks good to me
I suppose a more complete implementation of this could be The default behaviour without any options specified should match any of the 5 official and current versions of the standard. Here's how I would implement that: // declare 5 different sets with all the language codes in them
const sets = [isISO6391Set, isISO6392Set, isISO6393Set, isISO6394Set, isISO6395Set];
export default function isISO639(str, versions = [1, 2, 3, 4, 5]) {
assertString(str);
return versions.reduce((result, version) => {
const currentSet = sets[version - 1];
return currentSet.has(str) || result;
}, false);
} |
That would be a more complete implementation, but I'm not sure how often that would be used. Especially with the large number of supported languages for ISO 639-3, it might be too much for this project. |
I came looking for this feature today, any updates regarding this functionality? I agree with the option to specify the version via an options parameter, however, I would set the default version to 1. For now the default set in commit works for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objection from me. LGTM, thanks for your contrib 🎉
// Sorry for the delay in reviewing this.
This PR adds a validator which validates ISO 639-1 languages codes, such as
'af'
,'nb'
and'fr'
.I would like this feature because ISO 639-1 seems to be the most commonly used set of language codes when referring solely to a language and not a locale, and I will be using them for an upcoming project which uses
validator.js
.This PR is essentially a cleaned up version of a previous PR which I suspect was closed partially due to the lack of cleanup: #1103.
Thanks to @ezkemboi and @profnandaa for their contributions in earlier PRs and issues.
My contribution is the cleanup of the diff as well as rewritten test code and readme entry.
See related issue: #989
Checklist