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

Support boolean and number arrays #18

Closed
ulken opened this issue Apr 23, 2020 · 1 comment · Fixed by #19
Closed

Support boolean and number arrays #18

ulken opened this issue Apr 23, 2020 · 1 comment · Fixed by #19

Comments

@ulken
Copy link
Contributor

ulken commented Apr 23, 2020

Background
Currently, minimist-options supports type: array. However, the values of the array are always parsed as 'string'. yargs/yargs-parser (which are minimist backwards compatible) have support for coercing values 'number' and 'boolean' as well (see opts.array under yargs-parser API).

Proposal
Therefore, I would like to see minimist-options supporting these types too.

Considerations
I have considered the following options for how that could look:

  1. Add another property elementType: 'number'|'boolean' (potentially arrayElementType to tie it more to array). This would only be allowed when having type: array.
  2. Extend type with support for ['number'] and ['boolean'].
  3. Extend type with support for 'number-array' and 'boolean-array' (camelCasing possible if preferred).
  4. Extend type with support for numbers and booleans. Note: plural.
  5. Extend type with support for { type: 'array', elementType: 'number|boolean' }.

Note: Additionally, 'string' would of course be a valid element type.

#1 is explicit, but verbose, introduces an extra property which would only be valid under certain conditions (which is always a little "blah" and adds complexity).

#2 is a little magical, but I like how terse it is and find it quite expressive/intuitive when you look at it. ”Aha, it’s an array of (number|boolean)s”.

#3 would introduce magical (made-up) strings. Not very fond of.

#4 is probably the simplest of them all. My only concern is: is it distinct enough or too easy to mix up?

#5 Using an object is always neat, but having to specify type: 'array' again feels weird as it would be the only valid value, but leaving it out would be odd too.

I'm leaning towards #2 or #4.

Any other suggestions?

Backwards compatibility
To be backwards compatible and not break existing code bases, I suggest we still keep type: array and its behavior intact.

@vadimdemedes What say you?

@ulken
Copy link
Contributor Author

ulken commented Apr 25, 2020

@vadimdemedes Went ahead and implemented it. Hope you get around to have a look sometime soon. Would be much appreciated to have this merged (in some form). Thanks!

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.

1 participant