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

CLI::Option::envname() should trigger validators only if corresponding subcommand is called #903

Closed
w3lld0ne opened this issue Jul 9, 2023 · 1 comment · Fixed by #904

Comments

@w3lld0ne
Copy link

w3lld0ne commented Jul 9, 2023

Basic example:

auto CliApp = new CLI::App("myapp");
auto sub1 = CliApp->add_subcommand("sub1");
std::filesystem::path someFile = "";
sub1->add_option("-f,--file", someFile)->envname("SOME_FILE")->required()->expected(1)->check(CLI::ExistingFile);
auto sub2 = CliApp->add_subcommand("sub2");
int completelyUnrelatedToSub1 = 0;
sub2->add_option("-v,--value", completelyUnrelatedToSub1)->required()->expected(1);

try
{
	CliApp->parse(argsCount, args);
}
catch (const CLI::ParseError& e)
{
	return CliApp->exit(e);
}

Here if I call myapp.exe sub2 -v 111 and I have %SOME_FILE% env var defined as invalid/non-existing file path (it's not happening if there is no such env var defined), CLI::ExistingFile validator will abort the execution, even though I haven't even called the subcommand it's defined for. If I'd want this check to happen in sub2, I'd simply add the same option to it as well.

Even better: try to call myapp.exe (no arguments) or myapp.exe -h and we'll be greeted with the same validation error. It's funny because the error states Run with --help for more information. and that's what we do :) The only thing that works fine is myapp.exe -v.

If I remove ->envname("SOME_FILE") part from --file option registration (or delete %SOME_FILE% var from environment), everything works as it should: it's still marked as ->required(), but this is checked only for execution of sub1 subcommand.

P.S. I'm on Windows 11 22H2, compiling release v2.3.2 in VS2022 17.6.4 in C++20 mode. Will try the main branch soon.

@phlptp
Copy link
Collaborator

phlptp commented Jul 9, 2023

I will check on this, thanks.

phlptp added a commit that referenced this issue Jul 9, 2023
Do not check environment variables if a subcommand has not been
triggered.

Fixes #903

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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

Successfully merging a pull request may close this issue.

2 participants