search

jonschlinkert / randomatic

Easily generate random strings like passwords, with simple options for specifying a length and for using patterns of numeric, alpha-numeric, alphabetical, special or custom characters. (the original "generate-password")
https://github.com/jonschlinkert
MIT License
183 stars 25 forks source link

Should it throw an error on `randomatic('?', 4)`? #22

Open benfletcher opened 5 years ago

benfletcher commented 5 years ago

randomatic('?', 4)

Currently if the chars option is not specified, the characters from undefined are used as the custom characters (including the fact that d, n and e are 2x more frequent than the remaining letters.

Should an error be thrown instead? Or maybe [a-z] used and a message logged to that effect?

The readme does indeed specify that the default is undefined, but it was still a little surprising to me that it was allowed to be coerced to a string if not user-specified.

doowb commented 5 years ago

I think this is a bug in that opts.chars should be checked before doing mask += opts.chars.

Also, the readme does say specify to pass in the option when using ?:

?: Custom characters (pass a string of custom characters to the options)

I think we either throw an error or change this line to if (pattern.indexOf('?') !== -1) mask += (opts.chars || '');

I prefer the latter since it will fix the "bug" of undefined being used, but not change the API (e.g. throwing a new error) so we can do a patch bump.

A PR is welcome to make the change and update the .verb.md file with some information about what happens when ? is used and opts.chars is not set.