vercel / arg

Simple argument parsing
https://npmjs.com/arg
MIT License
1.23k stars 54 forks source link

Issues parsing negative numbers when passed without `=` #52

Open karfau opened 4 years ago

karfau commented 4 years ago

in node shell:

> const arg = require('arg')
undefined
> arg({"--int": parseInt}, {argv:["--int=-100"]})
{ _: [], '--int': -100 }
> arg({"--int": parseInt}, {argv:["--int -100"]})
Thrown:
{ Error: Unknown or unexpected option: --int -100
    at arg (/run/media/karfau/hdd-data/dev/node-cli-arguments-options/arg/node_modules/arg/index.js:88:19) code: 'ARG_UNKNOWN_OPTION' }
> arg({"--num": parseFloat}, {argv:["--num=-1.0"]})
{ _: [], '--num': -1 }
> arg({"--num": parseFloat}, {argv:["--num -1.0"]})
Thrown:
{ Error: Unknown or unexpected option: --num -1.0
    at arg (node-cli-arguments-options/arg/node_modules/arg/index.js:88:19) code: 'ARG_UNKNOWN_OPTION' }

I also tried to wrap the numbers with different kinds of quotes, but it didn't have any effect. I don't know if this is a "feature" or a "bug".

Update: The example above is bad in the sense that argv is different from what it would look like when using from the shell, which influences the error messages. A better example is:

> const arg = require('arg')
undefined
> arg({"--int": parseInt}, {argv:["--int", "-100"]})
Thrown:
Error: Option requires argument: --int
    at arg (node-cli-arguments-options/arg/node_modules/arg/index.js:105:13)
>  arg({"--float": parseFloat}, {argv:["--float", "-0.1"]})
Thrown:
Error: Option requires argument: --float
    at arg (node-cli-arguments-options/arg/node_modules/arg/index.js:105:13)

Since the repo doesn't seem to be maintained (sorry for that, it's not true, must have switched the name with one of the other repos), just filing this to make people aware since I discovered it while comparing features of different argument parsers (WIP)

Qix- commented 4 years ago

You have to pass arguments individually. --int -100 is two arguments.

arg({"--int": parseInt}, {argv: ["--int", "-100"]});

The only exception is --int=-100 because arg has special handling for long-arg (with the double-hyphen, e.g. --int) arguments with = in them (which it first splits on the first = and then continues parsing as two individual arguments).


Since it looks like you're creating a repository showing off many different argument parsing libraries, it's probably worth mentioning this is how all argument parsing is done unless the library accepts a full, single-string command line (which I would caution against anyone using, personally). This is because processes on most (all?) modern platforms accept command line arguments as individual strings fashioned into an array.

$ my_program foo bar "qux qix"
#include <stdio.h>

int main(int argc, char *argv[]) {
    for (int i = 0; i < argc; i++) {
        printf("%s\n", argv[i]);
    }
    return 0;
}
my_program
foo
bar
qux qix

In Node, process.argv is just the argv passed to int main(), the entry point. Nothing more.

It's up to the shell (cmd.exe, bash, fish, zsh, powershell, etc.) to split a single command line on the spaces and, depending on the shell's syntax, parse and then throw away any quotes.

For example, it's the shell (e.g. bash) keeping the foo bar in my_program --arg "foo bar" as a single argument, not the process/argument parser. That's because the shell is passing in foo bar as a single argument because the syntax of the shell's command line language (e.g. batch scripting on windows or shell scripting on linux) denotes double quotes as special characters.

That is also why double quotes never show up in your program's argv array - they are specific to shells, not to process arguments.

This is why you have to pass ["--int", "-100"] for the space-delimited form - basically doing what the shell would do for you (since arg doesn't use any shell parsing libraries).

karfau commented 4 years ago

Thx for the elaboration, it's true that I didn't (yet) check how other parsers behave on that aspect.

And it's true that I learn a lot during that research.

:+1:

karfau commented 4 years ago

FYI: I finally verified this behavior for command-line-args, commander and dashdash: none of them has this issue. And they are all similar in that they are "strict" (no unknown options by default) and each option either receives arguments or doesn't. (From the error message that arg prints, e.g. Option requires argument: --int, it looks as if it also made up it's mind about that fact, it's just not using that information when parsing the input.)

(And of course they don't expect you to pass in all options as a single string.) I will keep verifying this behavior for other libs, but I will most likely not reference this issue in commits in the future. (Interested people can check the current status in https://github.com/karfau/node-cli-arguments-options)

karfau commented 4 years ago

PS: I updated the initial description of the issue, since this is less about passing all args as a single string, it is about passing option arguments starting with - when the option requires an argument.

Qix- commented 4 years ago

Hi. I'm not sure what issue you're referring to. Very clearly in your original post, you had a single argument passed in expecting it to be parsed as two arguments. That's not what arg allows. If other libraries handle strange edge cases using magic, that's their own decision.

arg parses arguments akin to how they're parsed in lower-level languages on *nix platforms. It was created because these libraries are often bloated, magical, or slow. Therefore, it expects you to pass in arguments how the system is going to pass in arguments - separately.

I don't really know what to tell you here. There's no bug, arg is behaving in the correct way (as intended). If there is another issue you're facing, please elaborate, because it's not clear.

karfau commented 4 years ago

My intention was not to confuse you, but it seems to have happened anyways. Sorry for that. I'm also not really sure if I just didn't understand your API correctly.

I was trying to understand if there is a way to achieve the following: Writing a script that invoked like that: node script.js --int -5 would print {"_": [], "--int": -5}. (So far I wasn't able to achieve that using arg.) And if it's not possible and also not intended, it's totally fine for me and closing this ticket reflects this. I can also open a new issue for this if you prefer.

Best regards, karfau

Qix- commented 4 years ago

--int=-5 works. We don't have special handling for numbers. That is indeed a bug.

However, the confusion comes from your usage. In the OP, you CANNOT pass a single string ("--int -5") to the argv option property. That will never work, even with a bugfix.

I'll push a patch shortly. Thank you for bringing it up.

Qix- commented 4 years ago

Got distracted yesterday, apologies. Released as 4.1.3.

karfau commented 4 years ago

FYI: It currently only works when using type Number or BigInt. When using any method to "customize the number" like parseInt, it will still fail.

Qix- commented 4 years ago

Need to think of a good way to approach this problem, then.

karfau commented 4 years ago

If you want we can do it together.

My thoughts would be: If an option requires an argument the next thing in the list should be treated as such, independent of the first character. But I think the way you are doing the parsing is somehow different, so this might be a radical/breaking change.

Since I'm not familiar with your code base, feel free to explain how you do the parsing in general/differently (and why) if you want to exchange thoughts/arguments.

Qix- commented 4 years ago

That's why this is non-trivial; we check to see if the next argument starts with a -, and if it doesn't we error with an "option requires argument" error.

karfau commented 4 years ago

Maybe the proper way to handle it then is to document as loudly as possible something like "If an option argument could start with - use = to pass it."

If you could use a different criteria for the decision if something is an argument or an option (like a blacklist of types, instead of a white-list, or the type.length?), it might still not work to pass values that start with - if the option doesn't require an argument, right?

Not trying to make the problem more complex, just thinking out loud while trying to grasp the scope of it...

(If you would prefer a different channel for this communication, just let me know.)