CLIUtils / CLI11

CLI11 is a command line parser for C++11 and beyond that provides a rich feature set with a simple and intuitive interface.
https://cliutils.github.io/CLI11/book/
Other
3.21k stars 345 forks source link

Fix envname with validator bug introduced in v2.4.0 #1032

Open johanpel opened 3 months ago

johanpel commented 3 months ago

First of all, thank you for this project!

It looks like a bug was introduced in v2.4.0 where combining Option::envname with a validator through Option::check no longer properly fails to validate when the provided value is invalid.

For example, consider the following program:

#include <ostream>
#include <CLI/CLI.hpp>

int main(int argc, char **argv) {
    CLI::App app("Test");
    int test{0};
    app.add_option("-i", test)->envname("CLI11_ENVNAME_WITH_VALIDATOR_BUG")->check(CLI::Range(2, 10));
    CLI11_PARSE(app, argc, argv);
    std::cerr << test << std::endl;
    return 0;
}

Running this with ./test -i 1 correctly returns:

-i: Value 1 not in range [2 - 10]
Run with --help for more information.

However, when this is executed with CLI11_ENVNAME_WITH_VALIDATOR_BUG=1 ./test, this fails to error out on the invalid value, uses the default value, and returns:

0

In v2.3.1 this was still working as expected and correctly returns:

-i: Value 1 not in range [2 - 10]
Run with --help for more information.
w3lld0ne commented 2 months ago

Not a bug, but a change.

henryiii commented 2 months ago

CC @phlptp

phlptp commented 2 months ago

Having validators on environmental variables fail had the potential to cause issues with various other mechanics, including preventing help, version, or other short circuit mechanisms from operating with no user controllable way to bypass. To prevent that kind of issue, validator failures on environmental variables are treated as if the environmental variable was not present. This is not a perfect option, but I think had to be treated this way to prevent some more egregious usability failures. Some future designs for handling exceptions differently may resolve the conflict and allow multiple error conditions, but that will be a bigger effort.