From f2b993ebec10a66d7b71550b08b16a04056e93f2 Mon Sep 17 00:00:00 2001 From: baruch Date: Thu, 19 Oct 2017 14:52:24 +0300 Subject: [PATCH 1/3] Support required options --- include/clara.hpp | 118 ++++++++++++++++++++++++++++++--------------- src/ClaraTests.cpp | 9 +++- 2 files changed, 86 insertions(+), 41 deletions(-) diff --git a/include/clara.hpp b/include/clara.hpp index 2af4419..c26597e 100644 --- a/include/clara.hpp +++ b/include/clara.hpp @@ -450,9 +450,25 @@ namespace detail { class ParserBase { public: virtual ~ParserBase() = default; - virtual auto validate() const -> Result { return Result::ok(); } - virtual auto parse( std::string const& exeName, TokenStream const &tokens) const -> InternalParseResult = 0; - virtual auto cardinality() const -> size_t { return 1; } + virtual auto validateSettings() const -> Result { return Result::ok(); } + virtual auto validateFinal() const -> Result { return Result::ok(); } + virtual auto canParse() const -> bool { return false; } + virtual auto internalParse( std::string const& exeName, TokenStream const &tokens ) const->InternalParseResult = 0; + + auto parse( std::string const& exeName, TokenStream const &tokens ) const -> InternalParseResult { + auto validationResult = validateSettings(); + if( !validationResult ) + return InternalParseResult( validationResult ); + + auto result = internalParse( exeName, tokens ); + + // Call this even if parsing failed in order to perform cleanup + validationResult = validateFinal(); + if( result && result.value().type() != ParseResultType::ShortCircuitAll && !validationResult ) + return InternalParseResult( validationResult ); + + return result; + } auto parse( Args const &args ) const -> InternalParseResult { return parse( args.exeName(), TokenStream( args ) ); @@ -474,20 +490,26 @@ namespace detail { std::shared_ptr m_ref; std::string m_hint; std::string m_description; + mutable std::size_t m_count; - explicit ParserRefImpl( std::shared_ptr const &ref ) : m_ref( ref ) {} + explicit ParserRefImpl( std::shared_ptr const &ref ) + : m_ref( ref ), + m_count( 0 ) + {} public: template ParserRefImpl( T &ref, std::string const &hint ) : m_ref( std::make_shared>( ref ) ), - m_hint( hint ) + m_hint( hint ), + m_count( 0 ) {} template ParserRefImpl( LambdaT const &ref, std::string const &hint ) : m_ref( std::make_shared>( ref ) ), - m_hint(hint) + m_hint( hint ), + m_count( 0 ) {} auto operator()( std::string const &description ) -> DerivedT & { @@ -509,14 +531,27 @@ namespace detail { return m_optionality == Optionality::Optional; } - auto cardinality() const -> size_t override { + virtual auto cardinality() const -> size_t { if( m_ref->isContainer() ) return 0; else return 1; } + + auto validateFinal() const -> Result override { + if( !isOptional() && count() < 1 ) + return Result::runtimeError( "Missing token: " + hint() ); + m_count = 0; + return ComposableParserImpl::validateFinal(); + } + + auto canParse() const -> bool override { + return (cardinality() == 0 || count() < cardinality()); + } auto hint() const -> std::string { return m_hint; } + + auto count() const -> std::size_t { return m_count; } }; class ExeName : public ComposableParserImpl { @@ -541,7 +576,7 @@ namespace detail { } // The exe name is not parsed out of the normal tokens, but is handled specially - auto parse( std::string const&, TokenStream const &tokens ) const -> InternalParseResult override { + auto internalParse( std::string const&, TokenStream const &tokens ) const -> InternalParseResult override { return InternalParseResult::ok( ParseState( ParseResultType::NoMatch, tokens ) ); } @@ -565,11 +600,7 @@ namespace detail { public: using ParserRefImpl::ParserRefImpl; - auto parse( std::string const &, TokenStream const &tokens ) const -> InternalParseResult override { - auto validationResult = validate(); - if( !validationResult ) - return InternalParseResult( validationResult ); - + auto internalParse( std::string const &, TokenStream const &tokens ) const -> InternalParseResult override { auto remainingTokens = tokens; auto const &token = *remainingTokens; if( token.type != TokenType::Argument ) @@ -578,8 +609,10 @@ namespace detail { auto result = m_ref->setValue( remainingTokens->token ); if( !result ) return InternalParseResult( result ); - else + else { + ++m_count; return InternalParseResult::ok( ParseState( ParseResultType::Matched, ++remainingTokens ) ); + } } }; @@ -641,22 +674,19 @@ namespace detail { using ParserBase::parse; - auto parse( std::string const&, TokenStream const &tokens ) const -> InternalParseResult override { - auto validationResult = validate(); - if( !validationResult ) - return InternalParseResult( validationResult ); - + auto internalParse( std::string const &, TokenStream const &tokens ) const -> InternalParseResult override { auto remainingTokens = tokens; if( remainingTokens && remainingTokens->type == TokenType::Option ) { auto const &token = *remainingTokens; - if( isMatch(token.token ) ) { + if( isMatch( token.token ) ) { if( m_ref->isFlag() ) { auto result = m_ref->setFlag( true ); if( !result ) return InternalParseResult( result ); if( result.value() == ParseResultType::ShortCircuitAll ) return InternalParseResult::ok( ParseState( result.value(), remainingTokens ) ); - } else { + } + else { ++remainingTokens; if( !remainingTokens ) return InternalParseResult::runtimeError( "Expected argument following " + token.token ); @@ -669,13 +699,14 @@ namespace detail { if( result.value() == ParseResultType::ShortCircuitAll ) return InternalParseResult::ok( ParseState( result.value(), remainingTokens ) ); } + ++m_count; return InternalParseResult::ok( ParseState( ParseResultType::Matched, ++remainingTokens ) ); } } return InternalParseResult::ok( ParseState( ParseResultType::NoMatch, remainingTokens ) ); } - auto validate() const -> Result override { + auto validateSettings() const -> Result override { if( m_optNames.empty() ) return Result::logicError( "No options supplied to Opt" ); for( auto const &name : m_optNames ) { @@ -684,7 +715,7 @@ namespace detail { if( name[0] != '-' && name[0] != '/' ) return Result::logicError( "Option name must begin with '-' or '/'" ); } - return ParserRefImpl::validate(); + return ParserRefImpl::validateSettings(); } }; @@ -788,33 +819,42 @@ namespace detail { return os; } - auto validate() const -> Result override { + auto validateSettings() const -> Result override { for( auto const &opt : m_options ) { - auto result = opt.validate(); + auto result = opt.validateSettings(); if( !result ) return result; } for( auto const &arg : m_args ) { - auto result = arg.validate(); + auto result = arg.validateSettings(); if( !result ) return result; } return Result::ok(); } - using ParserBase::parse; + auto validateFinal() const -> Result override { + for( auto const &opt : m_options ) { + auto result = opt.validateFinal(); + if( !result ) + return result; + } + for( auto const &arg : m_args ) { + auto result = arg.validateFinal(); + if( !result ) + return result; + } + return Result::ok(); + } - auto parse( std::string const& exeName, TokenStream const &tokens ) const -> InternalParseResult override { + using ParserBase::parse; - struct ParserInfo { - ParserBase const* parser = nullptr; - size_t count = 0; - }; + auto internalParse( std::string const& exeName, TokenStream const &tokens ) const -> InternalParseResult override { const size_t totalParsers = m_options.size() + m_args.size(); - ParserInfo parseInfos[totalParsers]; + ParserBase const* parsers[totalParsers]; size_t i = 0; - for( auto const& opt : m_options ) parseInfos[i++].parser = &opt; - for( auto const& arg : m_args ) parseInfos[i++].parser = &arg; + for( auto const& opt : m_options ) parsers[i++] = &opt; + for( auto const& arg : m_args ) parsers[i++] = &arg; m_exeName.set( exeName ); @@ -822,14 +862,13 @@ namespace detail { while( result.value().remainingTokens() ) { bool tokenParsed = false; - for( auto& parseInfo : parseInfos ) { - if( parseInfo.parser->cardinality() == 0 || parseInfo.count < parseInfo.parser->cardinality() ) { - result = parseInfo.parser->parse(exeName, result.value().remainingTokens()); + for( auto& parser : parsers ) { + if( parser->canParse() ) { + result = parser->internalParse(exeName, result.value().remainingTokens()); if (!result) return result; if (result.value().type() != ParseResultType::NoMatch) { tokenParsed = true; - ++parseInfo.count; break; } } @@ -840,7 +879,6 @@ namespace detail { if( !tokenParsed ) return InternalParseResult::runtimeError( "Unrecognised token: " + result.value().remainingTokens()->token ); } - // !TBD Check missing required options return result; } }; diff --git a/src/ClaraTests.cpp b/src/ClaraTests.cpp index 83ba2fc..7f48094 100644 --- a/src/ClaraTests.cpp +++ b/src/ClaraTests.cpp @@ -118,7 +118,7 @@ TEST_CASE( "Combined parser" ) { ); } SECTION( "some args" ) { - auto result = parser.parse( Args{ "TestApp", "-n", "Bill", "-d:123.45", "-f", "test1", "test2" } ); + auto result = parser.parse( Args{ "TestApp", "-r", "42", "-n", "Bill", "-d:123.45", "-f", "test1", "test2" } ); CHECK( result ); CHECK( result.value().type() == ParseResultType::Matched ); @@ -127,6 +127,13 @@ TEST_CASE( "Combined parser" ) { REQUIRE( config.m_tests == std::vector { "test1", "test2" } ); CHECK( showHelp == false ); } + SECTION( "missing required" ) { + using namespace Catch::Matchers; + + auto result = parser.parse( Args{ "TestApp", "-n", "Bill", "-d:123.45", "-f", "test1", "test2" } ); + CHECK( !result ); + CHECK_THAT( result.errorMessage(), Contains( "Missing token" ) && Contains( "time|value" ) ); + } SECTION( "help" ) { auto result = parser.parse( Args{ "TestApp", "-?", "-n:NotSet" } ); CHECK( result ); From 1dcbe3cfd29152c1e5968a518ab001509864b6f1 Mon Sep 17 00:00:00 2001 From: Baruch Burstein Date: Sun, 5 Nov 2017 22:43:33 +0200 Subject: [PATCH 2/3] Reset counter before use, not after --- include/clara.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/clara.hpp b/include/clara.hpp index 6c9b1c8..b643b6d 100644 --- a/include/clara.hpp +++ b/include/clara.hpp @@ -545,11 +545,15 @@ namespace detail { else return 1; } + + auto validateSettings() const -> Result override { + m_count = 0; + return ComposableParserImpl::validateSettings(); + } auto validateFinal() const -> Result override { if( !isOptional() && count() < 1 ) return Result::runtimeError( "Missing token: " + hint() ); - m_count = 0; return ComposableParserImpl::validateFinal(); } From 5898faad3abfe4a564b3090efd80db7eea0eab7b Mon Sep 17 00:00:00 2001 From: Baruch Burstein Date: Sun, 12 Nov 2017 10:59:33 +0200 Subject: [PATCH 3/3] Fixed calling base class with template argument --- include/clara.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/clara.hpp b/include/clara.hpp index b643b6d..b01a5a1 100644 --- a/include/clara.hpp +++ b/include/clara.hpp @@ -548,13 +548,13 @@ namespace detail { auto validateSettings() const -> Result override { m_count = 0; - return ComposableParserImpl::validateSettings(); + return ComposableParserImpl::validateSettings(); } auto validateFinal() const -> Result override { if( !isOptional() && count() < 1 ) return Result::runtimeError( "Missing token: " + hint() ); - return ComposableParserImpl::validateFinal(); + return ComposableParserImpl::validateFinal(); } auto canParse() const -> bool override {