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

Add optional<T> data type to supported types #2

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

MiroPalmu
Copy link

@MiroPalmu MiroPalmu commented Jul 29, 2023

Have optional to be part of vat variant, where T can be int32_t, uin32_t, int64_t, uint64_t or std::string.

Example code using this feature:

auto input_a = std::optional<int>{ };
auto input_b = std::optional<std::string>{ };

const auto options = {
    { { "a" }, input_a, "Input a"},
    { { "b" }, input_b, "input_b"}
};

argz::parse(about, opts, argc, argv);

if (input_a) foo(input_a.value());
if (input_b) bar(input_b.value());

Notice that T can not be bool due to ambiguous meaning in the following example:

auto bool_opt = std::optional<bool>{ };

const auto options = {
    { { "a" }, bool_opt, "Input a"}
};

argz::parse(about, opts, argc, argv);

// Now if command line is: "./prog -a"
// should input_a be empty or contain true?

if (input_a) foo(input_a.value());

If optional would contain integer or string input_a would be empty optional.

@MiroPalmu
Copy link
Author

Force push was to remove redundant construction of std::reference_wrapper which got implicitly converted to normal reference immediately.

@stephenberry stephenberry merged commit 7e89591 into stephenberry:main Jul 31, 2023
3 checks passed
@stephenberry
Copy link
Owner

@MiroPalmu
This is brilliant! I made a tiny fix to build with clang. But, thanks so much for this addition.

@MiroPalmu
Copy link
Author

@stephenberry

I realized one complication about this. Templated/generic lambdas were added in c++20 and CMakeLists.txt at the moment has target_compile_features(argz_argz INTERFACE cxx_std_17)

This was not apparent when I was testing this because CMake somehow selects c++20 automatically for me.

@stephenberry
Copy link
Owner

@MiroPalmu
Ah, thanks for pointing this out. I'm fine moving to a C++20 requirement. All my other projects are moving to this requirement and I find C++20 to be widely available now. I'll update accordingly.

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.

2 participants