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

greedy argument parsing introduced by obviously evil ;) commit: 6669f0a #109

Closed
Byron opened this issue May 6, 2015 · 5 comments
Closed
Labels
C-bug Category: Updating dependencies

Comments

@Byron
Copy link
Contributor

Byron commented May 6, 2015

6669f0a introduces shorthand parsing for multiple values. However, this causes subcommands to be parsed as flag-values, thus making it impossible to properly call the program.

For example, calls like this ...

groupsmigration1 --scope foobar archive insert group -u simple README.md

... now fail as -u is not a valid argument. This is as --scope <url>... has eaten all arguments until the first -flag, then -u is interpreted as top-level flag which it is not. Usually archive and insert are subcommands, and group as well as -u are required flags of insert.

I have made a quick-fix (notes) which might not be ready for a PR, and I am unsure whether I am fit to fix this properly. Clearly we would want to add a few tests to assure this truly works and to protect from regression.

@kbknapp
Copy link
Member

kbknapp commented May 6, 2015

I would either set the number of values for --scope if you know how many, I'll also look at evaluating subcommands earlier so as to stop parsing --scope if a subcommand is reached. Currently it only stops if it reaches the max, or exact, number of expected values for --scope, or it reaches another argument starting with -. I can roll this up into #111 too

@Byron
Copy link
Contributor Author

Byron commented May 6, 2015

Unfortunately I don't - --scope is setup exactly how it needs to be, limiting it would be arbitrary. This issue stems from the simplification which allows me to also specify tiny multi-flags like -r foo=bar snoo=baz only once, which is appreciated too.
But if I'd have to choose, I would choose correct parsing over this convenience.
On the other hand, a proper implementation seems possible, it's just that values for + multi-value flags must not be subcommand names anymore.
If there really is no way, I would probably just make --scope single-value, which is what it had to be in the old-ages of docopt, which had the same problem. Interestingly, argparse in python has the same issue too, and I explicitly hacked my own copy to get rid of that limitation back in the days.

@kbknapp
Copy link
Member

kbknapp commented May 6, 2015

No worries, it should be fixed with #111 Once it's merged I'll update crates.io (0.8.1 on crates.io)

@kbknapp kbknapp mentioned this issue May 6, 2015
@kbknapp kbknapp closed this as completed in fc79017 May 6, 2015
@kbknapp kbknapp added the C-bug Category: Updating dependencies label May 6, 2015
@kbknapp
Copy link
Member

kbknapp commented May 6, 2015

The newest version should solve the issue for you, and it was a valid bug.

To expand on the issue though; the only thing I can't currently do (and realistically it's probably impossible to do) is tell when a [unlimited] multiple value stops and a positional starts. Because positionals have no specifier (unless you count -- which works, but it also marks all following arguments after that as positional as well). The answer is to either rearrange the arguments (so the positional comes before the multiple values), or use --.

@Byron
Copy link
Contributor Author

Byron commented May 6, 2015

Thank you ! I have rolled-back my change locally and could verify it works fine.

Regarding the stated problem with positionals (and grammars like prog <x> <y> -m <v>...) which are difficult to parse unless in the right order: I think we just have no technology advanced enough just yet.
Well, actually, it is there, just not for this field: regular expressions. The engine behind that is able to act non-greedily, while maximizing matches for respective terms.
I know no implementation using regex though - maybe because for the most part, arg-parsers could get away with relatively trivial (compared to a regex system) and manual implementations.

Now that I am implementing a simple json lexer to allow me to write a filter to get rid of null values in json streams, I am also thinking that this kind of pattern matching would be totally suited to any kind of token-based system. However, a non-text based regular expression engine is also nowhere to be found. Such a thing would certainly be overkill for what I want to do, but I am lazy and would prefer to write a simple substitution regex which works on my json tokens.

Anyway, thanks again for all the fixes - I believe I am totally happy now, with the CLI at least :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants