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

'string' value_type for options not converted when number is passed #55

Open
johnkchiu opened this issue Oct 16, 2014 · 2 comments · May be fixed by #59
Open

'string' value_type for options not converted when number is passed #55

johnkchiu opened this issue Oct 16, 2014 · 2 comments · May be fixed by #59

Comments

@johnkchiu
Copy link

I set a option as 'string':

...
        account: ['a', 'account number', 'string']
...

And this works well in most cases EXCEPT when the value is a number:

$ ./script.js -a 12345

In this case, typeof (options.account) is number. Can you do a covert to String when you do a cli.getValue(...)?

Thanks!

@johnkchiu
Copy link
Author

Actually, looks like there's even more problems with 'string' handling. When the value is in int format, it's always getting parsed as int, so the value is getting truncated:

$ ./script.js -a 1312868311701071952
debug: [20223:index.js]  account=1312868311701072000, file=null, max=null

Problem is here - https://github.com/chriso/cli/blob/master/cli.js#L737. It needs to check that 'value_type' is not 'string'.

@julianbrowne
Copy link

Looks like that conditional at https://github.com/chriso/cli/blob/master/cli.js#L737 is actually redundant. Commenting it out and testing with:

cli.parse({ 
    astring: ['s', 'an int as a string', 'string'],
    anint:   ['i', 'a (possibly rounded) integer', 'int']
});

and

-s 523465823658273456834632487 -i 6453246587365873568

works fine, because actual ints (as opposed to strings that contain only numbers) are parsed via getInt at https://github.com/chriso/cli/blob/master/cli.js#L757 using a regex.

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 a pull request may close this issue.

2 participants