Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
johanpel opened this issue Apr 16, 2024 · 3 comments
Open

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

johanpel opened this issue Apr 16, 2024 · 3 comments

Comments

@johanpel
Copy link

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
Copy link

Not a bug, but a change.

@henryiii
Copy link
Collaborator

CC @phlptp

@phlptp
Copy link
Collaborator

phlptp commented Apr 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants