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

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

Open
benfletcher opened this issue Feb 19, 2019 · 1 comment
Open

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

benfletcher opened this issue Feb 19, 2019 · 1 comment
Labels

Comments

@benfletcher
Copy link

benfletcher commented Feb 19, 2019

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 doowb added the bug label Feb 19, 2019
@doowb
Copy link
Collaborator

doowb commented Feb 19, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants