CLI11 icon indicating copy to clipboard operation
CLI11 copied to clipboard

Fix envname with validator bug introduced in v2.4.0

Open johanpel opened this issue 2 years ago • 3 comments

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.

johanpel avatar Apr 16 '24 15:04 johanpel

Not a bug, but a change.

w3lld0ne avatar Apr 29 '24 18:04 w3lld0ne

CC @phlptp

henryiii avatar Apr 29 '24 19:04 henryiii

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.

phlptp avatar Apr 30 '24 14:04 phlptp