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

Allow check if ArgumentParser has parsed values #218

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

skrobinson
Copy link
Contributor

No description provided.

This allows checking whether user input was processed into the parser
or any attached subparsers.

Closes p-ranav#212

Signed-off-by: Sean Robinson <[email protected]>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

test/test_bool_operator.cpp Show resolved Hide resolved
@p-ranav p-ranav merged commit ed2953a into p-ranav:master Oct 11, 2022
@skrobinson skrobinson deleted the feat-bool-argparser branch October 12, 2022 14:22
return it.second;
});

return m_is_parsed && (arg_used || subparser_used);
Copy link
Contributor

@SergiusTheBest SergiusTheBest Nov 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skrobinson Why don't we use just m_is_parsed? I have a subcommand without arguments and operator bool() returns false for it.

@skrobinson
Copy link
Contributor Author

@SergiusTheBest The question I'm trying to answer with this operator bool() is: has this parser (or it's subparsers) run and found values. Using only m_is_parsed would only indicate the parser ran without an error, not that any values were parsed.

For the case where subparsers are commands and don't require any arguments the ArgumentParser::is_subcommand_used() function may better match your needs.

@SergiusTheBest
Copy link
Contributor

@skrobinson I understand. But it's not so obvious as operator bool() is very generic and is better suited for m_is_parsed than m_is_parsed && (arg_used || subparser_used) (taking into account that arguments are optional and not required for a command).

@SergiusTheBest
Copy link
Contributor

@skrobinson BTW, is a test case required for contributing? I'd like to add ArgumentParser::is_subcommand_used(const ArgumentParser& subcommand).

@skrobinson
Copy link
Contributor Author

Test cases are encouraged. You could probably add to and adapt tests in test/test_subparsers.cpp rather than writing new tests.

I believe your new overload is a good idea.

@SergiusTheBest
Copy link
Contributor

Ok, thanks for the hint.

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 this pull request may close these issues.

3 participants