jarro2783/cxxopts

my_program -DFOO=bar fails to parse

Closed this issue · 15 comments

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.

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.

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.

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.

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.

Actually = is only allowed for long options.

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

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

I added an = here:

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

it appears to work:

src/example -ffile=baz
Files
file=baz

@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^%$#@\

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

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.

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' 

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

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

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

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