Commit 4d5bff2393eae8348e7abdada6af3f4e5e075593
Committed by
Henry Schreiner
1 parent
afd4e328
Adding ArgumentMismatch, changable improvement
Showing
4 changed files
with
32 additions
and
35 deletions
include/CLI/App.hpp
| @@ -274,8 +274,6 @@ class App { | @@ -274,8 +274,6 @@ class App { | ||
| 274 | 274 | ||
| 275 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 275 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 276 | CLI::callback_t fun = [&variable, simple_name](CLI::results_t res) { | 276 | CLI::callback_t fun = [&variable, simple_name](CLI::results_t res) { |
| 277 | - if(res.size() != 1) | ||
| 278 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 279 | return detail::lexical_cast(res[0], variable); | 277 | return detail::lexical_cast(res[0], variable); |
| 280 | }; | 278 | }; |
| 281 | 279 | ||
| @@ -293,8 +291,6 @@ class App { | @@ -293,8 +291,6 @@ class App { | ||
| 293 | 291 | ||
| 294 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 292 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 295 | CLI::callback_t fun = [&variable, simple_name](CLI::results_t res) { | 293 | CLI::callback_t fun = [&variable, simple_name](CLI::results_t res) { |
| 296 | - if(res.size() != 1) | ||
| 297 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 298 | return detail::lexical_cast(res[0], variable); | 294 | return detail::lexical_cast(res[0], variable); |
| 299 | }; | 295 | }; |
| 300 | 296 | ||
| @@ -325,7 +321,7 @@ class App { | @@ -325,7 +321,7 @@ class App { | ||
| 325 | }; | 321 | }; |
| 326 | 322 | ||
| 327 | Option *opt = add_option(name, fun, description, false); | 323 | Option *opt = add_option(name, fun, description, false); |
| 328 | - opt->set_custom_option(detail::type_name<T>(), -1, true); | 324 | + opt->set_custom_option(detail::type_name<T>(), -1); |
| 329 | return opt; | 325 | return opt; |
| 330 | } | 326 | } |
| 331 | 327 | ||
| @@ -347,7 +343,7 @@ class App { | @@ -347,7 +343,7 @@ class App { | ||
| 347 | }; | 343 | }; |
| 348 | 344 | ||
| 349 | Option *opt = add_option(name, fun, description, defaulted); | 345 | Option *opt = add_option(name, fun, description, defaulted); |
| 350 | - opt->set_custom_option(detail::type_name<T>(), -1, true); | 346 | + opt->set_custom_option(detail::type_name<T>(), -1); |
| 351 | if(defaulted) | 347 | if(defaulted) |
| 352 | opt->set_default_str("[" + detail::join(variable) + "]"); | 348 | opt->set_default_str("[" + detail::join(variable) + "]"); |
| 353 | return opt; | 349 | return opt; |
| @@ -454,9 +450,6 @@ class App { | @@ -454,9 +450,6 @@ class App { | ||
| 454 | 450 | ||
| 455 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 451 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 456 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { | 452 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { |
| 457 | - if(res.size() != 1) { | ||
| 458 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 459 | - } | ||
| 460 | bool retval = detail::lexical_cast(res[0], member); | 453 | bool retval = detail::lexical_cast(res[0], member); |
| 461 | if(!retval) | 454 | if(!retval) |
| 462 | throw ConversionError("The value " + res[0] + "is not an allowed value for " + simple_name); | 455 | throw ConversionError("The value " + res[0] + "is not an allowed value for " + simple_name); |
| @@ -480,9 +473,6 @@ class App { | @@ -480,9 +473,6 @@ class App { | ||
| 480 | 473 | ||
| 481 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 474 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 482 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { | 475 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { |
| 483 | - if(res.size() != 1) { | ||
| 484 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 485 | - } | ||
| 486 | bool retval = detail::lexical_cast(res[0], member); | 476 | bool retval = detail::lexical_cast(res[0], member); |
| 487 | if(!retval) | 477 | if(!retval) |
| 488 | throw ConversionError("The value " + res[0] + "is not an allowed value for " + simple_name); | 478 | throw ConversionError("The value " + res[0] + "is not an allowed value for " + simple_name); |
| @@ -509,9 +499,6 @@ class App { | @@ -509,9 +499,6 @@ class App { | ||
| 509 | 499 | ||
| 510 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 500 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 511 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { | 501 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { |
| 512 | - if(res.size() != 1) { | ||
| 513 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 514 | - } | ||
| 515 | member = detail::to_lower(res[0]); | 502 | member = detail::to_lower(res[0]); |
| 516 | auto iter = std::find_if(std::begin(options), std::end(options), [&member](std::string val) { | 503 | auto iter = std::find_if(std::begin(options), std::end(options), [&member](std::string val) { |
| 517 | return detail::to_lower(val) == member; | 504 | return detail::to_lower(val) == member; |
| @@ -541,9 +528,6 @@ class App { | @@ -541,9 +528,6 @@ class App { | ||
| 541 | 528 | ||
| 542 | std::string simple_name = CLI::detail::split(name, ',').at(0); | 529 | std::string simple_name = CLI::detail::split(name, ',').at(0); |
| 543 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { | 530 | CLI::callback_t fun = [&member, options, simple_name](CLI::results_t res) { |
| 544 | - if(res.size() != 1) { | ||
| 545 | - throw ConversionError("Only one " + simple_name + " allowed"); | ||
| 546 | - } | ||
| 547 | member = detail::to_lower(res[0]); | 531 | member = detail::to_lower(res[0]); |
| 548 | auto iter = std::find_if(std::begin(options), std::end(options), [&member](std::string val) { | 532 | auto iter = std::find_if(std::begin(options), std::end(options), [&member](std::string val) { |
| 549 | return detail::to_lower(val) == member; | 533 | return detail::to_lower(val) == member; |
include/CLI/Error.hpp
| @@ -43,6 +43,7 @@ enum class ExitCodes { | @@ -43,6 +43,7 @@ enum class ExitCodes { | ||
| 43 | InvalidError, | 43 | InvalidError, |
| 44 | HorribleError, | 44 | HorribleError, |
| 45 | OptionNotFound, | 45 | OptionNotFound, |
| 46 | + ArgumentMismatch, | ||
| 46 | BaseClass = 127 | 47 | BaseClass = 127 |
| 47 | }; | 48 | }; |
| 48 | 49 | ||
| @@ -146,6 +147,17 @@ class RequiredError : public ParseError { | @@ -146,6 +147,17 @@ class RequiredError : public ParseError { | ||
| 146 | CLI11_ERROR_SIMPLE(RequiredError) | 147 | CLI11_ERROR_SIMPLE(RequiredError) |
| 147 | }; | 148 | }; |
| 148 | 149 | ||
| 150 | +/// Thrown when the wrong number of arguments has been recieved | ||
| 151 | +class ArgumentMismatch : ParseError { | ||
| 152 | + CLI11_ERROR_DEF(ParseError, ArgumentMismatch) | ||
| 153 | + ArgumentMismatch(std::string name, int expected, size_t recieved) | ||
| 154 | + : ArgumentMismatch(expected > 0 ? ("Expected exactly " + std::to_string(expected) + " arguments to " + name + | ||
| 155 | + ", got " + std::to_string(recieved)) | ||
| 156 | + : ("Expected at least " + std::to_string(-expected) + " arguments to " + name + | ||
| 157 | + ", got " + std::to_string(recieved)), | ||
| 158 | + ExitCodes::ArgumentMismatch) {} | ||
| 159 | +}; | ||
| 160 | + | ||
| 149 | /// Thrown when a requires option is missing | 161 | /// Thrown when a requires option is missing |
| 150 | class RequiresError : public ParseError { | 162 | class RequiresError : public ParseError { |
| 151 | CLI11_ERROR_DEF(ParseError, RequiresError) | 163 | CLI11_ERROR_DEF(ParseError, RequiresError) |
include/CLI/Option.hpp
| @@ -207,14 +207,15 @@ class Option : public OptionBase<Option> { | @@ -207,14 +207,15 @@ class Option : public OptionBase<Option> { | ||
| 207 | 207 | ||
| 208 | /// Set the number of expected arguments (Flags bypass this) | 208 | /// Set the number of expected arguments (Flags bypass this) |
| 209 | Option *expected(int value) { | 209 | Option *expected(int value) { |
| 210 | - if(value == 0) | 210 | + if(expected_ == value) |
| 211 | + return this; | ||
| 212 | + else if(value == 0) | ||
| 211 | throw IncorrectConstruction("Cannot set 0 expected, use a flag instead"); | 213 | throw IncorrectConstruction("Cannot set 0 expected, use a flag instead"); |
| 212 | - else if(expected_ == 0) | ||
| 213 | - throw IncorrectConstruction("Cannot make a flag take arguments!"); | ||
| 214 | else if(!changeable_) | 214 | else if(!changeable_) |
| 215 | throw IncorrectConstruction("You can only change the expected arguments for vectors"); | 215 | throw IncorrectConstruction("You can only change the expected arguments for vectors"); |
| 216 | else if(last_) | 216 | else if(last_) |
| 217 | throw IncorrectConstruction("You can't change expected arguments after you've set take_last!"); | 217 | throw IncorrectConstruction("You can't change expected arguments after you've set take_last!"); |
| 218 | + | ||
| 218 | expected_ = value; | 219 | expected_ = value; |
| 219 | return this; | 220 | return this; |
| 220 | } | 221 | } |
| @@ -447,7 +448,11 @@ class Option : public OptionBase<Option> { | @@ -447,7 +448,11 @@ class Option : public OptionBase<Option> { | ||
| 447 | results_t partial_result = {results_.back()}; | 448 | results_t partial_result = {results_.back()}; |
| 448 | local_result = !callback_(partial_result); | 449 | local_result = !callback_(partial_result); |
| 449 | } else { | 450 | } else { |
| 450 | - local_result = !callback_(results_); | 451 | + if((expected_ > 0 && results_.size() != static_cast<size_t>(expected_)) || |
| 452 | + (expected_ < 0 && results_.size() < static_cast<size_t>(-expected_))) | ||
| 453 | + throw ArgumentMismatch(single_name(), expected_, results_.size()); | ||
| 454 | + else | ||
| 455 | + local_result = !callback_(results_); | ||
| 451 | } | 456 | } |
| 452 | 457 | ||
| 453 | if(local_result) | 458 | if(local_result) |
| @@ -527,13 +532,13 @@ class Option : public OptionBase<Option> { | @@ -527,13 +532,13 @@ class Option : public OptionBase<Option> { | ||
| 527 | /// @name Custom options | 532 | /// @name Custom options |
| 528 | ///@{ | 533 | ///@{ |
| 529 | 534 | ||
| 530 | - /// Set a custom option, typestring, expected, and changeable | ||
| 531 | - void set_custom_option(std::string typeval, int expected = 1, bool changeable = false) { | 535 | + /// Set a custom option, typestring, expected; locks changeable unless expected is -1 |
| 536 | + void set_custom_option(std::string typeval, int expected = 1) { | ||
| 532 | typeval_ = typeval; | 537 | typeval_ = typeval; |
| 533 | expected_ = expected; | 538 | expected_ = expected; |
| 534 | if(expected == 0) | 539 | if(expected == 0) |
| 535 | required_ = false; | 540 | required_ = false; |
| 536 | - changeable_ = changeable; | 541 | + changeable_ = expected < 0; |
| 537 | } | 542 | } |
| 538 | 543 | ||
| 539 | /// Set the default value string representation | 544 | /// Set the default value string representation |
tests/AppTest.cpp
| @@ -119,7 +119,7 @@ TEST_F(TApp, DualOptions) { | @@ -119,7 +119,7 @@ TEST_F(TApp, DualOptions) { | ||
| 119 | EXPECT_EQ(ans, vstr); | 119 | EXPECT_EQ(ans, vstr); |
| 120 | 120 | ||
| 121 | args = {"--string=one", "--string=two"}; | 121 | args = {"--string=one", "--string=two"}; |
| 122 | - EXPECT_THROW(run(), CLI::ConversionError); | 122 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 123 | } | 123 | } |
| 124 | 124 | ||
| 125 | TEST_F(TApp, LotsOfFlags) { | 125 | TEST_F(TApp, LotsOfFlags) { |
| @@ -737,7 +737,7 @@ TEST_F(TApp, FailSet) { | @@ -737,7 +737,7 @@ TEST_F(TApp, FailSet) { | ||
| 737 | app.add_set("-q,--quick", choice, {1, 2, 3}); | 737 | app.add_set("-q,--quick", choice, {1, 2, 3}); |
| 738 | 738 | ||
| 739 | args = {"--quick", "3", "--quick=2"}; | 739 | args = {"--quick", "3", "--quick=2"}; |
| 740 | - EXPECT_THROW(run(), CLI::ConversionError); | 740 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 741 | 741 | ||
| 742 | app.reset(); | 742 | app.reset(); |
| 743 | 743 | ||
| @@ -770,7 +770,7 @@ TEST_F(TApp, InSetIgnoreCase) { | @@ -770,7 +770,7 @@ TEST_F(TApp, InSetIgnoreCase) { | ||
| 770 | 770 | ||
| 771 | app.reset(); | 771 | app.reset(); |
| 772 | args = {"--quick=one", "--quick=two"}; | 772 | args = {"--quick=one", "--quick=two"}; |
| 773 | - EXPECT_THROW(run(), CLI::ConversionError); | 773 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 774 | } | 774 | } |
| 775 | 775 | ||
| 776 | TEST_F(TApp, VectorFixedString) { | 776 | TEST_F(TApp, VectorFixedString) { |
| @@ -1145,27 +1145,24 @@ TEST_F(TApp, CheckSubcomFail) { | @@ -1145,27 +1145,24 @@ TEST_F(TApp, CheckSubcomFail) { | ||
| 1145 | EXPECT_THROW(CLI::detail::AppFriend::parse_subcommand(&app, args), CLI::HorribleError); | 1145 | EXPECT_THROW(CLI::detail::AppFriend::parse_subcommand(&app, args), CLI::HorribleError); |
| 1146 | } | 1146 | } |
| 1147 | 1147 | ||
| 1148 | -// Added to test defaults on dual method | ||
| 1149 | TEST_F(TApp, OptionWithDefaults) { | 1148 | TEST_F(TApp, OptionWithDefaults) { |
| 1150 | int someint = 2; | 1149 | int someint = 2; |
| 1151 | app.add_option("-a", someint, "", true); | 1150 | app.add_option("-a", someint, "", true); |
| 1152 | 1151 | ||
| 1153 | args = {"-a1", "-a2"}; | 1152 | args = {"-a1", "-a2"}; |
| 1154 | 1153 | ||
| 1155 | - EXPECT_THROW(run(), CLI::ConversionError); | 1154 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 1156 | } | 1155 | } |
| 1157 | 1156 | ||
| 1158 | -// Added to test defaults on dual method | ||
| 1159 | TEST_F(TApp, SetWithDefaults) { | 1157 | TEST_F(TApp, SetWithDefaults) { |
| 1160 | int someint = 2; | 1158 | int someint = 2; |
| 1161 | app.add_set("-a", someint, {1, 2, 3, 4}, "", true); | 1159 | app.add_set("-a", someint, {1, 2, 3, 4}, "", true); |
| 1162 | 1160 | ||
| 1163 | args = {"-a1", "-a2"}; | 1161 | args = {"-a1", "-a2"}; |
| 1164 | 1162 | ||
| 1165 | - EXPECT_THROW(run(), CLI::ConversionError); | 1163 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 1166 | } | 1164 | } |
| 1167 | 1165 | ||
| 1168 | -// Added to test defaults on dual method | ||
| 1169 | TEST_F(TApp, SetWithDefaultsConversion) { | 1166 | TEST_F(TApp, SetWithDefaultsConversion) { |
| 1170 | int someint = 2; | 1167 | int someint = 2; |
| 1171 | app.add_set("-a", someint, {1, 2, 3, 4}, "", true); | 1168 | app.add_set("-a", someint, {1, 2, 3, 4}, "", true); |
| @@ -1175,14 +1172,13 @@ TEST_F(TApp, SetWithDefaultsConversion) { | @@ -1175,14 +1172,13 @@ TEST_F(TApp, SetWithDefaultsConversion) { | ||
| 1175 | EXPECT_THROW(run(), CLI::ConversionError); | 1172 | EXPECT_THROW(run(), CLI::ConversionError); |
| 1176 | } | 1173 | } |
| 1177 | 1174 | ||
| 1178 | -// Added to test defaults on dual method | ||
| 1179 | TEST_F(TApp, SetWithDefaultsIC) { | 1175 | TEST_F(TApp, SetWithDefaultsIC) { |
| 1180 | std::string someint = "ho"; | 1176 | std::string someint = "ho"; |
| 1181 | app.add_set_ignore_case("-a", someint, {"Hi", "Ho"}, "", true); | 1177 | app.add_set_ignore_case("-a", someint, {"Hi", "Ho"}, "", true); |
| 1182 | 1178 | ||
| 1183 | args = {"-aHi", "-aHo"}; | 1179 | args = {"-aHi", "-aHo"}; |
| 1184 | 1180 | ||
| 1185 | - EXPECT_THROW(run(), CLI::ConversionError); | 1181 | + EXPECT_THROW(run(), CLI::ArgumentMismatch); |
| 1186 | } | 1182 | } |
| 1187 | 1183 | ||
| 1188 | // Added to test ->transform | 1184 | // Added to test ->transform |