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

Issues parsing negative numbers when passed without = #52

Open
karfau opened this issue Jan 10, 2020 · 13 comments
Open

Issues parsing negative numbers when passed without = #52

karfau opened this issue Jan 10, 2020 · 13 comments

Comments

@karfau
Copy link

karfau commented Jan 10, 2020

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)

karfau added a commit to karfau/node-cli-arguments-options that referenced this issue Jan 10, 2020
`{"features":24,"specified":7,"verified":17,"checks":36}`
- some additions in arg/dump.js as suggested in #6

Filed vercel/arg#52
@Qix-
Copy link
Contributor

Qix- commented Jan 10, 2020

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).

@Qix- Qix- added the question label Jan 10, 2020
@Qix- Qix- closed this as completed Jan 10, 2020
@Qix- Qix- added the invalid label Jan 10, 2020
@karfau
Copy link
Author

karfau commented Jan 10, 2020

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.

👍

karfau added a commit to karfau/node-cli-arguments-options that referenced this issue Jan 31, 2020
While working on documenting dashdash features I discovered that it is able
to parse negative values just fine.
This is also true for command-line-args.
The assumption that this would not be possible in general originated from the referenced issue. And I remember I checked at least one other lib after that reply.
I will now add those cases to all compared libs.

vercel/arg#52
karfau added a commit to karfau/node-cli-arguments-options that referenced this issue Feb 1, 2020
Added "custom option handling" for integer and number.

vercel/arg#52
karfau added a commit to karfau/node-cli-arguments-options that referenced this issue Feb 1, 2020
Just added the case from the ticket, so it's more obvious.

vercel/arg#52
@karfau
Copy link
Author

karfau commented Feb 1, 2020

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
Copy link
Author

karfau commented Feb 1, 2020

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-
Copy link
Contributor

Qix- commented Feb 1, 2020

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
Copy link
Author

karfau commented Feb 1, 2020

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-
Copy link
Contributor

Qix- commented Feb 1, 2020

--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- Qix- reopened this Feb 1, 2020
@Qix- Qix- closed this as completed in 07628f1 Feb 1, 2020
@Qix-
Copy link
Contributor

Qix- commented Feb 2, 2020

Got distracted yesterday, apologies. Released as 4.1.3.

@karfau
Copy link
Author

karfau commented Feb 9, 2020

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- Qix- reopened this Feb 9, 2020
@Qix-
Copy link
Contributor

Qix- commented Feb 9, 2020

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

@karfau
Copy link
Author

karfau commented Feb 9, 2020

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-
Copy link
Contributor

Qix- commented Feb 9, 2020

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
Copy link
Author

karfau commented Feb 9, 2020

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.)

nevilm-lt pushed a commit to nevilm-lt/arg that referenced this issue Apr 22, 2022
himanshiLt pushed a commit to himanshiLt/arg that referenced this issue Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants