vercel / arg

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

Separate `--` and `_` arguments #30

Closed blakeembrey closed 5 years ago

blakeembrey commented 5 years ago

I need the ability to get _ arguments and -- separately. Currently everything after -- gets placed into args._ so I can't tell which was after the -- argument or which was part of the command. This is required to use in projects such as https://github.com/Qard/onchange.

Qix- commented 5 years ago

I'm :-1: on this, this isn't how -- is supposed to work. Seems like you're trying to fix an XY problem - can you expand a bit on what you're trying to do by splitting the parsed arguments?

blakeembrey commented 5 years ago

@Qix- It's how the library works: https://github.com/Qard/onchange.

blakeembrey commented 5 years ago

I would, of course, be open to hear how this should work instead of how it does work. Using -- seems like a reasonable use-case here to me. But since this is an existing library and existing feature in other libraries like minimist it would be nice to have - even if you say it shouldn't be done.

Qix- commented 5 years ago

I don't like the argument of "other libraries do it, therefore..." because a lot of other libraries have large scopes in order to make everyone that has a niche use case happy 😅 Which is a large reason why arg was written.

For what it's worth, this pattern of option parsing isn't common in the UNIX world.

Some examples of programs that tackle this in different ways are:

-- is the canonical way to say "stop reading the rest of the CLI as options/flags and treat them as positionals". That is the mental contract users have when using CLI's.


The only way I'd be comfortable handling this is to have an option stopAtDashes: true or something like that, akin to stopAtPositional. But I feel strongly against having a separate array for arguments.

P.S. excuse any double dots, I know they look really snarky but Apple makes dodgy keyboards lately and some keys tend to hit twice. Currently, my period key is affected. I think I edited all of them out 😅

blakeembrey commented 5 years ago

@Qix- To be honest, I agree and it's not really an argument at all for supporting this (that would be a bad argument). The argument is that this is an existing use-case and the change does not significantly increased complexity or types (it adds around 5 lines to handle -- separately inside an if handler['--']). I do understand if you don't want to support this in any case because it's "non-unix-like", in which case we can update the README.md or CONTRIBUTING.md and indicate non-unix-like behaviour is discouraged.

It does seem like I could refactor onchange to use stopAtPositional going forward, move current pattern usage behind -p or something and release a new major version to use arg. This just seemed like a nice compromise for today's library as many people understand how it works already.

FWIW, I avoided making it a config option because it makes the type system weird - just using opts['--'] was super nice because it required zero type system changes, things "just work".

blakeembrey commented 5 years ago

@Qix- Also, for the find style behaviour, there's no way to do this in arg today. Do you want to try and support this somehow? Maybe a separate symbol for "sub-command" parsing?

Qix- commented 5 years ago

Yeah I'm trying to think of how this could be added in a generally GNU-ish way but I can't think of anything that doesn't introduce yet another option. :/ It's a real edge case IMO, I don't see many programs use -- as a major delimiter.

Also, for the find style behaviour, there's no way to do this in arg today. Do you want to try and support this somehow?

Not really, no - their flags are positional as well (as in, order of flags matters), and they use single hyphens for long flags. It's an entirely different parsing scheme that would more than likely be easier to implement in a custom manner - definitely out of scope for arg.

Qix- commented 5 years ago

Actually, a new option might be okay in this case:

{
   literalDashes: true
}

That would cause -- to be treated as a positional, allowing you to split ._ how you'd like. It would also solve a few other use-cases as well.

Further, it plays nicely with stopAtPositional, which is what I was worried about.

blakeembrey commented 5 years ago

This might work but it’s still an incompatible change for onchange since there’s no specific order required today for the patterns and CLI args. I don’t think it’s incompable with this suggestion either so I don’t think I should block you if it helps other cases!

Qix- commented 5 years ago

Could you explain your use case then? With the change in #35, you can achieve what you want here in this ticket.

blakeembrey commented 5 years ago

@Qix- It's not compatible because onchange "**/*.ts" -k -- npm start would mean something else in #35 compared to #33 (#35 would stop parsing when you hit "**/*.ts" instead of --).

Qix- commented 5 years ago

No, #35 doesn't stop parsing. It just includes -- in the ._ array instead of treating it specially. However, you're right - it would then try to parse any arguments after the -- as flags as per the spec object passed to arg, which presents a new problem for you.

In onchange, what I would personally do is just to .slice() based on process.argv since it's a really niche use-case, and then just run the first of the two arrays through arg. Plus, that will make more sense looking at the code.

const dashIdx = process.argv.indexOf('--');
if (dashIdx === -1) {
    throw new Error('no program arguments given');
}

const programArgs = process.argv.slice(dashIdx);
const args = arg(
    {
        /* ... */
    }, {
        argv: argv.slice(2, dashIdx)
    }
);

It's untested, but that should do it, and IMO it makes it very clear to me (the reader) what's happening.

blakeembrey commented 5 years ago

@Qix- Sure, that makes sense 👍