Skip to content

my_program -DFOO=bar fails to parse #357

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

Closed
eyalroz opened this issue Aug 8, 2022 · 15 comments · Fixed by #360
Closed

my_program -DFOO=bar fails to parse #357

eyalroz opened this issue Aug 8, 2022 · 15 comments · Fixed by #360

Comments

@eyalroz
Copy link
Contributor

eyalroz commented Aug 8, 2022

I'm running the following program:

#include <cxxopts/cxxopts.hpp>
#include <vector>
#include <string>

int main(int argc, char** argv)
{
    cxxopts::Options options { argv[0], "test program"};
    options.add_options()
        ("D,define", "description here", cxxopts::value<std::vector<std::string>>())
        ;
    options.parse(argc, argv);
}

and, when building with the master branch (or v3.0.0), I get this:

$ ./my_program -DA_LITTLE_EXTRA=1
terminate called after throwing an instance of 'cxxopts::option_syntax_exception'
  what():  Argument ‘-DA_LITTLE_EXTRA=1’ starts with a - but has incorrect syntax
Aborted

Despite the fact that, supposedly, #158 is fixed.

@jarro2783
Copy link
Owner

I'm not sure if that's supposed to parse. The = is treated as an attached value, so I guess you want this to support

-D "A_LITTLE_EXTRA=1"

but with the -D attached to its argument.
It might work by just adding an = to the allowed values.

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 9, 2022

The = should not be treated as an attached value, because the value starts at the 'A'. So, where would I add the =? I don't think adding it to option_matcher does the trick. Also, it's not just the =; the value can have any character. A sequence beginning with - and an alnum should be parsed as group flags until the first flag which needs a value, after which the rest of the sequence should count as its flags.

@jarro2783
Copy link
Owner

Well the whole value is A_LITTLE_EXTRA=1 isn't it? The option is -D, and the rest is the value. I'm just not sure how easy it is to distinguish this from something like -ab=foo, as in the equivalent of -a -b foo, where b takes an argument.

@jarro2783
Copy link
Owner

Actually = is only allowed for long options. It could be added to short options as an allowed character and just matched like any other character.

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 10, 2022

Actually = is only allowed for long options.

Didn't quite understand this sentence. "allowed" - by you? in principle? in the current implementation?

@jarro2783
Copy link
Owner

I mean allowed by the current implementation. I'll try something and see if I can get it to work.

@jarro2783
Copy link
Owner

jarro2783 commented Aug 10, 2022

I added an = here:

-  ("--([[:alnum:]][-_[:alnum:]]+)(=(.*))?|-([[:alnum:]]+)");
+  ("--([[:alnum:]][-_[:alnum:]]+)(=(.*))?|-([[:alnum:]|=]+)");

it appears to work:

src/example -ffile=baz
Files
file=baz

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 10, 2022

@jarro2783 : That specific exception doesn't cut it. Any sequence of characters (other than '\0') should be acceptable after the -D, not just ([[:alnum:]|=]+). I happened to give an example with = but that's no more than an example. And this would be just like for a -- option: after the = (or in the next argument), all characters are allowed. So we should see (with shell escapes on the input):

$ example -f=--f=-ff^%\$#@\\
Files
file==--f=-ff^%$#@\

@jarro2783
Copy link
Owner

That sounds reasonable. I'll get that working and make sure nothing else is broken.

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 10, 2022

Great. But - remember that:

  1. This should fail if D is a flag which doesn't take a value.
  2. You also have to account for option grouping. That is, after a '-' and without a second '-', you need to match flags until the first flag which takes a value, and only after that comes its value.

@jarro2783
Copy link
Owner

Yeah I thought of that, I checked my first patch and this already works:

src/example -bffile=baz
Saw option ‘b’
Files
file=baz

and when it's not recognised it just treats everything as options

src/example -DFOO=BAR
Arguments remain = 2
Saw 0 arguments
Unmatched options: '-D' '-F' '-O' '-O' '-=' '-B' '-A' '-R' 

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 11, 2022

@jarro2783 : I'm just anxious to close this issue...

@jarro2783
Copy link
Owner

I have a fix that I will finish off in the next day.

@eyalroz
Copy link
Contributor Author

eyalroz commented Aug 12, 2022

So, it was that simple after all? No issues with option grouping?

@jarro2783
Copy link
Owner

No, given that only the last option in a group of single options could actually take a value, the parsing already worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants