Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

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

Open
bearcage opened this issue Nov 6, 2017 · 11 comments
Open

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

bearcage opened this issue Nov 6, 2017 · 11 comments

Comments

@bearcage
Copy link

bearcage commented Nov 6, 2017

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
Copy link
Author

bearcage commented Nov 6, 2017

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
Copy link

aldanor commented Dec 8, 2017

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

@da2r-20
Copy link

da2r-20 commented Nov 26, 2018

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
Copy link

rpavlik commented May 1, 2019

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

@mattgodbolt
Copy link

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

@grafikrobot
Copy link

If it's any consolation .required() is now actually required in Lyra thanks to this PR bfgroup/Lyra@8d6f122

@rpavlik
Copy link

rpavlik commented Aug 14, 2019

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.)

@rpavlik
Copy link

rpavlik commented Aug 14, 2019

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

@grafikrobot
Copy link

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

@rpavlik
Copy link

rpavlik commented Aug 14, 2019

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants