Allow for earlier exit with non matching options

[levi-de-koning]

During testing, we encountered an unexpected error message. Let me introduce this small sample to explain the error message and how it confused us.

CLI::App testApp;
int value;
testApp.add_option("--foo", value);
std::string file;
testApp.add_option("file", file)->check(CLI::ExistingFile);

CLI11_PARSE(testApp);

We will use -bar 5 somefile.txt for the command line arguments. We can already see this command line is invalid because -bar is not a valid option. When we run this, we get the error: file: File does not exist: 5. To a user, this is a super unexpected error as well. They specified some file.txt as their filename. Now, let's say we remove this validator. The error changes to The following arguments were not expected: --bar. This is the actual logical error I would expect for this command line.

The underlying reason for this is that. --bar will fail to match any subcommand and is added to the missing/remaining list. then 5 is parsed next, and this matches the position argument, which now fails the validation.

So, in conclusion, it would be nice if we could specify that any non-matching argument should throw an error. I don't necessarily think this should be the default, as you would lose the ability to get a list of all options that do not match. but at least being able to configure this would greatly clarify the error messages for cases like this.

[phlptp]

can you try this with the validate_positionals modifier and see if the error is the same. My guess is that will do what you want in this case.

[levi-de-koning]

can you try this with the validate_positionals modifier and see if the error is the same. My guess is that will do what you want in this case.

Indeed that should work in this example I believe, however if you where to combine it with a positional argument consuming all arguments like https://github.com/CLIUtils/CLI11/blob/main/examples/arg_capture.cpp

it should lead to to the the argument in this case being matched to the final consuming argument if I am understanding this correctly:

.validate_positionals(): Specify that positionals should pass validation before matching. Validation is specified through transform, check, and each for an option. If an argument fails validation it is not an error and matching proceeds to the next available positional or extra arguments.

[phlptp]

It requires positionals to have validation on them, yes.
In the example given you are using the remaining to capture all the arguments after a particular prefix command. which forces everything after that to be a positional.

So I am still a little unclear under what scenarios cannot be handled appropriately with some of the different options between the prevalidation, and trigger_on_parse modifiers

[levi-de-koning]

It requires positionals to have validation on them, yes. In the example given you are using the remaining to capture all the arguments after a particular prefix command. which forces everything after that to be a positional.

So I am still a little unclear under what scenarios cannot be handled appropriately with some of the different options between the prevalidation, and trigger_on_parse modifiers

Hey, sorry for the late reply; work has been very hectic.
To make the example a bit more representative I updated it to the following.

CLI::App testApp;
int value;
testApp.add_option("--foo", value);
std::string file;
std::vector<std::string> args;
testApp.add_option("file", file)->check(CLI::ExistingFile);
testApp.add_option("args", args);
testApp.positionals_at_end();

CLI11_PARSE(testApp);

So when the commandline --bar 5 somefile.txt is given the following will happen:

1. --bar is not found on TestApp and it's moved to the missing list in App::_move_to_missing
2. next 5 is parsed, as we don't know if --bar is a flag or an option we cannot simply ignore it.
3. the value 5 could however be a valid positional so it's tried with file where the validator denies it due to it not being a file, throwing a warning.

This produces the undesirable warning off file: File does not exist: 5

From my understanding of the code, CLI only throws a parse error at the very end in App::_process_extras(), so in this case, I would expect an error of type ExtrasError at a much earlier stage in the pipeline immediately as it's detected.

To come back a bit on why. validate_positionals wouldn't quite solve this, as there is an argument at the end that consumes everything without any condition, which can lead to values simply being moved to this final positional.

I locally made the following change:

CLI11_INLINE void App::_move_to_missing(detail::Classifier val_type, const std::string &val) {
+   if(!allow_extras_) {
+       throw ExtrasError(name_, {val});
+  }
    
   if(allow_extras_ || subcommands_.empty()) {
        missing_.emplace_back(val_type, val);
        return;
    }
    // allow extra arguments to be places in an option group if it is allowed there
    for(auto &subc : subcommands_) {
        if(subc->name_.empty() && subc->allow_extras_) {
            subc->missing_.emplace_back(val_type, val);
            return;
        }
    }
    // if we haven't found any place to put them yet put them in missing
    missing_.emplace_back(val_type, val);
}

This ensures that if allow_extras is disabled it imediatly throws an error. the downside being ofcourse you cannot get a list of all arguments that are missing.