search

catchorg / Clara

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

.required() on Parsers doesn't make them fail the parse #39

Open bearcage opened 6 years ago

bearcage commented 6 years ago

I'm guessing this is just not written yet, since the spot where it'd go at the end of parse() has a // !TBD about it. It doesn't look like there's a super clean place to expose required/not to users of the API, since there's no magic operator()/operator[] left open for it, in contrast with descriptions/flags (unless you want to get cute with overloading), but even just correct behavior with the somewhat-less-pretty .required() would work for me.

If you've got an approach in mind, I'd be happy to take a whack at it since it looks like it won't take too much to do.

cc @TheOnlyRew

bearcage commented 6 years ago

Oh, hey, looks like #28 is already addressing this, so I'll just leave the issue here for now. If either of you need a hand with this just let me know.

aldanor commented 6 years ago

Also just bumped into this, and #28 seems to have been hanging there for a while now.

da2r-20 commented 5 years ago

Was it fixed? I'm using v1.1.5 and it still doesn't fail on a non-give required() Arg|Opt

Update: checked master and it still reproduces

rpavlik commented 5 years ago

So #28 got replaced, apparently, with another patch that didn't actually do this?

mattgodbolt commented 5 years ago

+1 this is super confusing...I made an Arg required and apart from the help updating, it didn't actually make it required :)

grafikrobot commented 5 years ago

If it's any consolation .required() is now actually required in Lyra thanks to this PR https://github.com/bfgroup/Lyra/commit/8d6f122f33041557435383761286a9dbe292c68e

rpavlik commented 5 years ago

Can that commit just be ported over to this repo and submitted as a PR?

(There's #94 but it seems less nice than this one.)

grafikrobot commented 5 years ago

https://github.com/catchorg/Clara#-this-repository-is-unmaintained-go-here-for-a-fork-that-is-somewhat-maintained-

rpavlik commented 5 years ago

Hmm. Doesn't solve the "this is used in catch2" problem though.

grafikrobot commented 5 years ago

I don't see where required() is used in catch2. So why is it a problem?

rpavlik commented 5 years ago

Because I'm working on a test suite with a custom main function in catch2, and I'm trying to do what is recommended there and use Clara to parse my custom arguments. I do have required arguments.