dthree / vorpal

Node's framework for interactive CLIs
http://vorpal.js.org
MIT License
5.63k stars 278 forks source link

Added command option to disable type-casting #268

Open laurent22 opened 7 years ago

laurent22 commented 7 years ago

By default, the underlying command parser package (minimist), converts strings that look like numbers to actual numbers. This causes all kind of problems since it's also going to randomly convert things like hashes to numbers.

For example, a hash like "a1ef" would result in "a1ef", but another hash like "93e8" would result in "9300000000". There are quite a few pull requests about this in the minimist package GitHub page, but unfortunately it's no longer maintained so it looks like it won't be fixed.

This pull request attempts to fix this in a way that preserves backward compatibility. It adds a command option disableTypeCasting() which, when used, disables type casting for all arguments and all flags for that command. If that option is not set, the code path is exactly the same so the applications that don't explicitly opt-in won't be affected.

laurent22 commented 7 years ago

Is there any chance this pull request could be merged in one form or another?

I keep running into issues due to Minimist automatic casting. For example, most of the time the arguments are going to be strings so it's fine to use functions like "indexOf" on them, but in rare cases the argument is going to be converted to a number, which makes string functions suddenly fail.

So I need to cast to strings every time when getting an argument from Vorpal. You basically can't expect any consistency and It's quite a annoying bug that can remain undetected for a long time in a program.

Is there maybe some changes I could make to the pull request to get it accepted?

milesj commented 7 years ago

@laurent22 The master branch is currently in development for v2, so merging this in and tagging a new release isn't possible at this time. We're also planning to remove minimist all together.

milesj commented 7 years ago

I moved 2.0 code to a branch and reset master. Sorry, but you'll need to rebase or start this PR again.

laurent22 commented 7 years ago

Ok that's good news actually if you are going to remove minimist. Is there any roadmap for v2.0? (just to get some idea of what's coming and maybe contribute)

milesj commented 7 years ago

There's a very high level overview here: https://github.com/dthree/vorpal/issues/234

And the actual refactor PR here: https://github.com/dthree/vorpal/pull/272

troggy commented 6 years ago

any workaround? I would like to preserve my input as string, not number

troggy commented 6 years ago

For those looking for solution. This will make all arguments of the command to be parsed as strings:

.types({
  string: ['_']
})