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

m_throwOnUnrecognisedTokens should be true by default #6

Closed vadz closed 6 years ago

vadz commented 8 years ago

IMO silently discarding unknown options is not the right behaviour and I think it should be changed before really making the library public.

vadz commented 8 years ago

Things seem to be even worse here than I thought: even after calling setThrowOnUnrecognisedTokens(), unaccounted positional parameters are still silently ignored instead of resulting in an error. Can this be really intentional or is it a bug?

Using parse() (and not parseInto()), I can't even check for this from outside the library!

koliyo commented 6 years ago

This setting is not even available now? This functionality is crucial for proper command line error handling imo. Silently dropping misspelled options is not a good approach.

philsquared commented 6 years ago

Sorry for the silence on this - I'm trying to get back on top of things here. As you, hopefully, know Clara was rewritten from the ground up for C++11 (and to be "composable").

Initially I missed handling of unrecognised options in the rewrite, but added that in shortly after - so it's been in for a while now.

The rewritten version does not include setThrowOnUnrecognisedTokens() as that was a workaround intended to allow you to run two parsers in serial (so if the options weren't supported by the first one, they might be by the second). This was far from ideal, and is the main motivation for the composability in the re-write. You can now just combine parsers into a single composite parser - so it becomes appropriate for it to detect unrecognised options and return an error. (See this lightning talk for more)

And here is, likely, where the problem is. I haven't done enough, yet, to make this clear! Clara is trying to move away from using exceptions (it currently still does in one place). Instead I use monadic result types. So when you call parse() you get a result object back which, in the case of an error, contains the error message, which you can check and print out.

See the Catch usage for an example of how to handle this.

I need to document this properly - and ideally do a bit more work to make it more convenient, too.

Sorry for the confusion!