catchorg / Clara

A simple to use, composable, command line parser for C++ 11 and beyond
Boost Software License 1.0
650 stars 67 forks source link

Treat lambdas as containers #23

Open BMBurstein opened 7 years ago

BMBurstein commented 7 years ago

It would be useful if options that use lambdas would allow the same option being specified multiple times.

philsquared commented 7 years ago

Good point. The infrastructure is in there to handle it. I just need to join up the dots - thanks for bringing it up.

KingDuckZ commented 6 years ago

In order to achieve this, I made my own ListOpt class that inherits from clara::Opt and overrides cardinality() to always return 0. Unfortunately when I compose ListOpt | clara::Opt etc... the objects composed this way end up being stored by value in some std::vector container, which slices the objects and causes my override to be lost. Given that clara::Opt is not marked as final, and that clara::Help inherits from it, this looks like a bug to me.

KingDuckZ commented 6 years ago

Is there any update on this? Slicing is a serious bug and I'm afraid I might encounter other strange behaviour as I use Clara.

philsquared commented 6 years ago

Thanks for the ping, @KingDuckZ. I think the correct approach here is to make them final. I'm now struggling to remember exactly what the original issue @BMBurstein raises was, though (looks like it was obvious to me at the time). Do you want to rebind to the same lambda?

BMBurstein commented 6 years ago

If I remember correctly, I wanted lambdas to be treated as a container, allowing the option to be specified multiple times on the command line, with the lambda being called multiple times

philsquared commented 6 years ago

ah yes - that makes sense now! Thanks

KingDuckZ commented 6 years ago

I think it's fine to have it final (though that would break your Help class). My only regret about that is that this really looks like the way to achieve the container semantics on lambdas. But I feel your pain in turning every item in the internal vector into a pointer. One alternative I can think of is to implement the virtual call yourself: by adding a function pointer (not std::function) into Opt and making it point to Opt::cardinality() you should achieve just that. Then you'll have to take some optional parameters at construction time to give client code the opportunity to customise the fake virtual pointer. Ugly to the outside world but it should work at the modest price of increasing Opt's size by 1 pointer. Or, you could take the cardinality as an int at construction time altogether, though that will not allow for dynamic cardinalities (which is fine I guess). Or maybe you can come up with a better idea, since you have a deeper knowledge about the system.

I'm sure I worked around all that somehow, but I really can't remember how. Even reading at the code, I can't see anything fancy, so it's possible I just hacked your code to work in my specific case :p

philsquared commented 6 years ago

yeah, Help would need some special treatment - but shouldn't be a major obstacle. In fact it was a dynamic hierarchy to begin with. I changed it to the current design as part of an effort to reduce or eliminate dynamic memory allocations (there's still plenty to go) - which some of the embedded people were asking for. It seemed to make sense since it's only really Opts and Args that go in there (even Help is just a type of "constructor" for Opt, although a dangerous one, as it would be easy to forget about the slicing, so I think I need to deal with that anyway).

So I need to do a bit more big picture thinking about the best way to deal with all this (including the multi-invokeable lambdas)