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

What if actions want to return std::vector? #69

Closed
zhihaoy opened this issue Dec 1, 2019 · 6 comments
Closed

What if actions want to return std::vector? #69

zhihaoy opened this issue Dec 1, 2019 · 6 comments

Comments

@zhihaoy
Copy link
Contributor

zhihaoy commented Dec 1, 2019

If an option wants to parse a list of values --foo 1,4,2,6, then it cannot return std::vector<int> because this type is considered a container. We probably will never want to parse a list like that (because there are so many good parsing libraries for structural data), so we cannot return the existing containers functionality to "make get<std::vector<T>>() work" for this demand.

We should consider changing the multivalue argument support. Some observations:

  • Blessing types takes away possible return types
  • std::vector<std::string> is verbose to spell
  • std::array is not supported (and is tricky to support, because it's not a container that can grow)
  • std::list is probably never used for lots of reasons

Inventing a new set of API is an option, but the present API (#66) will double the number of names. I think the situation can be improved by blessing a type that is both impossible to be stored in std::any, and is terse to write -- array type.

Here is how the new get may look like:

  • get<T[]>: returns std::vector<T>
  • get<T[N]>: returns std::array<T, N> if N == nargs, throws otherwise
  • get<T>: returns T

The old spelling get<std::vector<std::string>> becomes get<std::string[]> after the change. To not to break everything immediately, we can make the old spelling [[deprecated]].

FAQ

  1. Why T[] and T[N] return entirely different types? I hope users can interpret [] as "returning a type with operator[]."
  2. Why not other kinds of containers? If users want they can append to anything with void actions. We are providing a good default here.
  3. Why not std::span (of static extent or dynamic extent)? Technical issue, we only have storage for std::vector<std::any> rather than std::vector<T>.
@p-ranav
Copy link
Owner

p-ranav commented Dec 2, 2019

Actions were designed to be similar to the visitor pattern in ANTLR - visit each argument/fragment, make changes given some context and save it. The parameter to .action() is a const string& representing each fragment, e.g.,

$./program --foo 1 4 3 6

Here, action receives 1, then 4, then 3, and finally 6. Inside the visitor, I can convert each fragment to whatever type I want, e.g., MyStringWrapper(arg) and return it.

How does this change in your proposal? What is the argument to .action(). Is it now a vector of strings if nargs > 1?

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Dec 2, 2019

I know this part, but what if users want to do this:

program.action([](std::string const& s) {
   return std::vector<int>(std::stoi(s));  // let's say it's a vector of N random numbers
});

Then the user cannot use program.get<std::vector<int>>("..."); to get the value, because that ends up in the container case, and the value type is deemed int. The "real" value type is std::vector<int>, so bad_any_cast will be thrown.

@p-ranav
Copy link
Owner

p-ranav commented Dec 2, 2019

Understood. Just need some clarifications:

What is the argument s in your example? Let's say the use-case is --foo 1 2 3 4

Then is s == 1? What is the action returning? The vector {1} then the vector {2} and so on?

.. Or is s == "1 2 3 4" which is the whole argument and the action returns {1, 2, 3, 4}? - This is clearly not the case per your example but it might be the intention.

Just need to clarify the semantics here.

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Dec 2, 2019

s is "1" in --foo 1. It's "1" then "2" in --foo 1 2 but I don't meant to bring in nargs > 1. It's an nargs == 1 argument, but wants to return a std::vector.

@p-ranav
Copy link
Owner

p-ranav commented Dec 2, 2019

Ah I see now. Okay, makes sense.

@p-ranav
Copy link
Owner

p-ranav commented Sep 21, 2022

Parsing a list of values is now supported with .nargs(...). See here for more details. Closing this now.

@p-ranav p-ranav closed this as completed Sep 21, 2022
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

2 participants