Unknown option gives user unfriendly error

[solid-angle]

If you misspell or provide an option that doesn't have a parser, when at least one argument is present in the parser, the returned error code isn't very user friendly.

Easiest to show with an example.

Given this code

#include <lyra/lyra.hpp>
#include <filesystem>
#include <iostream>

int main(int argc, char** argv)
{
    bool showHelp=false;
    bool verbose = false;
    auto cli = lyra::cli_parser();

    cli.add_argument(lyra::help(showHelp));
    cli.add_argument(lyra::opt(verbose)
        .name("--verbose").help("Verbose logging").optional());

    std::filesystem::path projectDirectory;
    cli.add_argument(lyra::arg(projectDirectory, "ProjectDirectory")
        .required().help("Project directory"));

    auto result = cli.parse({ argc, argv });

    if (!result)
    {
        std::cerr << result.errorMessage() << std::endl;
        return 1;
    }

    std::cout << projectDirectory.string() << std::endl;
    return 0;
}

and the commandline arguments

--foo /my/project/directory

The returned error is

Unrecognized token: /my/project/directory

Expected: Parser should recognize the '--' or '-' prefix on '--foo' and give an error about the missing option.

[grafikrobot]

After some investigation... This is technically not a bug. The current behavior is that it interprets the --foo as the value for the ProjectDirectory argument after having it not match the know options. Subsequently it sees the /my/project/directory and can't match it to anything, as it's not a known option, and it already parsed the one argument. Hence fails the parse at that point. And complains about it.

I'll have to think about it more to see if there's a way to deal with this use case.

[vadz]

It might well be not a bug technically, but this behaviour is very surprising and unwelcome. In practice this happens when the user makes a typo in the option name, e.g. suppose your program synopsis is a.out [--overwrite] <file>.... Running it as ./a.out --overrite file really ought to give an error about an unknown option rather than creating an output file called --overrite!

(and, of course, if you really wanted to create a file called like this, you should be able to use ./a.out -- --overrite file)

[grafikrobot]

@vadz indeed, it's annoying. Which is why I haven't closed this :-) Addressing this is not easy because it break backwards compat. But I'm thinking that okay in this case. Just will take more thinking to minimize the impact.

[vadz]

Would you be interested in a PR implementing this? I'd like to finally do something about it, because as much as I like Lyra's API, we can't tell our users never to make typos in their command lines (spoiler: it doesn't work).

[grafikrobot]

@vadz if you can think of a way to do it I'd love to have a fix for it!

[roelandschoukens]

Yeah I also think this is quite undesirable.

I had a crack at solving it by detecting the "--" token in the token iterator, the iterator can then stop returning anything after calling option().

#101

I don’t know if this change covers everything but at least the tests are passing. I did hit an annoying problem with negative numbers, but this is a known annoying problem with POSIX style arguments. What do you think of this change?