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

[parser::params] add overload accepting std::initializer_list #80

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kedartal
Copy link

@kedartal kedartal commented May 16, 2023

Allows calling cmdl.params( { "--name", "-n" } ) and getting all relevant values, regardless of which param name was used on the command line. For example,

std::ranges::for_each(
   cmdl.params( { "--name", "-n" } ) | std::views::values,
   []( const std::string& value ) {
      // do something with the parameter value
    } );

Since the implementation uses std::ranges, perhaps it's a good idea to conditionally enable it only for c++>=20 and feature test __cpp_lib_ranges; I know some projects prefer making equivalent tests in the build system and setting a compiler definition accordingly, any thoughts?

Also, this implementation requires that the parser's lifetime exceeds that of the return value -- this gets captured in the lambda -- which to me seems natural in most use cases, and a reasonable thing to require (and document). The alternative would be making a copy of the data...

@kedartal
Copy link
Author

Conditionally enabling the overload based on std::ranges availability.

__has_include is standard for c++>=17, #include <version> provides the __cpp_lib_ranges definition.

@adishavit
Copy link
Owner

Thank you for this - I have a super busy period - and won't be able to properly review.
Please be patient - I'll get to it.

That being said, I will say that Argh is intentionally C++11 (some original C++14 functionality/code was actively downgraded/backported) to allow broader appeal.
A C++11 conforming implementation is more likely to be accepted.

OTOH, I am considering a new version >= C++20, to allow more powerful features - bit that's still ways off.

@kedartal
Copy link
Author

Cool, let me know if I can help with the modern version when you get around to it.

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.

None yet

2 participants