-
-
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
isAlpha enhancement #1286
isAlpha enhancement #1286
Changes from 10 commits
d5291cf
7b198ca
e9bfc4e
5ff4998
5f385de
8d4dff5
d432f49
7629bde
9cd236a
a919d99
d313ed6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,8 +1,22 @@ | ||||||||
import assertString from './util/assertString'; | ||||||||
import { alpha } from './alpha'; | ||||||||
|
||||||||
export default function isAlpha(str, locale = 'en-US') { | ||||||||
assertString(str); | ||||||||
export default function isAlpha(_str, locale = 'en-US', options = {}) { | ||||||||
assertString(_str); | ||||||||
|
||||||||
let str = _str; | ||||||||
const { ignore } = options; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am with the opinion that Cc. @profnandaa There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm technically i don't see a possibility of failing for any of the above args, may be i can add those validator.js/src/lib/isAlpha.js Lines 8 to 10 in a919d99
the above line says that ignore is optional which means, as far as about this let me know if it makes sense @profnandaa @ezkemboi |
||||||||
|
||||||||
if (ignore) { | ||||||||
if (ignore instanceof RegExp) { | ||||||||
str = str.replace(ignore, ''); | ||||||||
} else if (typeof ignore === 'string') { | ||||||||
str = str.replace(new RegExp(`[${ignore.replace(/[-[\]{}()*+?.,\\^$|#\\s]/g, '\\$&')}]`, 'g'), ''); // escape regex for ignore | ||||||||
} else { | ||||||||
throw new Error('ignore should be instance of a String or RegExp'); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
if (locale in alpha) { | ||||||||
return alpha[locale].test(str); | ||||||||
} | ||||||||
|
Large diffs are not rendered by default.
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.
I am just curious why change
str
to_str
and later doI thought we could pass
str
as before and do:But, all in all, it looks good to 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.
@ezkemboi ah it's not good practice to directly change the arguments, just following best practices, will resolve the conflict in sometime