[Fix] Command line arg parser was not properly parsing negative numbe…

…r values (#179) Negative numbers were treated as new options.
olympus-robotics · Sep 30, 2024 · 254e892 · 254e892
1 parent bc1ff5b
commit 254e892
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 6 deletions.
diff --git a/modules/cli/src/program_options.cpp b/modules/cli/src/program_options.cpp
@@ -5,6 +5,7 @@
 #include "hephaestus/cli/program_options.h"
 
 #include <algorithm>
+#include <cctype>
 #include <cstdio>  // NOLINT(misc-include-cleaner)
 #include <cstdlib>
 #include <sstream>
@@ -96,10 +97,10 @@ auto ProgramDescription::parse(const std::vector<std::string>& args) && -> Progr
     ++arg_it;
     throwExceptionIf<InvalidParameterException>(
         arg_it == args.end(), fmt::format("After option --{} there is supposed to be a value", option.key));
+    const auto is_option = arg_it->starts_with('-') && arg_it->size() > 1 && std::isdigit((*arg_it)[1]) == 0;
     throwExceptionIf<InvalidParameterException>(
-        arg_it->starts_with('-'),
-        fmt::format("Option --{} is supposed to be followed by a value, not another option {}", option.key,
-                    *arg_it));
+        is_option, fmt::format("Option --{} is supposed to be followed by a value, not another option {}",
+                               option.key, *arg_it));
 
     option.value = *arg_it;
     option.is_specified = true;

diff --git a/modules/cli/tests/program_options_tests.cpp b/modules/cli/tests/program_options_tests.cpp
@@ -28,10 +28,11 @@ TEST(ProgramOptions, Option) {
       .defineOption<std::string>("other_option", 'o', "other desc")
       .defineOption<float>("bar", "desc", NUMBER)
       .defineOption<int>("foo", 'f', "desc", 1)
+      .defineOption<float>("baz", "desc")
       .defineFlag("flag", 'b', "desc");
   {
-    auto options =
-        ProgramDescription{ desc }.parse({ "--option", "value", "-o", "other_value", "--bar", "1.2" });
+    auto options = ProgramDescription{ desc }.parse(
+        { "--option", "value", "-o", "other_value", "--bar", "1.2", "--baz", "-1.0" });
     EXPECT_TRUE(options.hasOption("option"));
     EXPECT_EQ(options.getOption<std::string>("option"), "value");
     EXPECT_TRUE(options.hasOption("other_option"));
@@ -40,12 +41,14 @@ TEST(ProgramOptions, Option) {
     EXPECT_EQ(options.getOption<float>("bar"), 1.2f);
     EXPECT_TRUE(options.hasOption("foo"));
     EXPECT_EQ(options.getOption<int>("foo"), 1);
+    EXPECT_TRUE(options.hasOption("baz"));
+    EXPECT_EQ(options.getOption<float>("baz"), -1.0);
     EXPECT_TRUE(options.hasOption("flag"));
     EXPECT_FALSE(options.getOption<bool>("flag"));
   }
   {
     auto options = ProgramDescription{ desc }.parse(
-        { "--option", "value", "-o", "other_value", "--bar", "1.2", "--flag" });
+        { "--option", "value", "-o", "other_value", "--bar", "1.2", "--baz", "-20", "--flag" });
     EXPECT_TRUE(options.hasOption("flag"));
     EXPECT_TRUE(options.getOption<bool>("flag"));
   }
@@ -68,6 +71,7 @@ TEST(ProgramOptions, Errors) {
     EXPECT_THROW(ProgramDescription{ desc }.parse({ "--option", "value", "other_value" }),
                  InvalidParameterException);
     EXPECT_THROW(ProgramDescription{ desc }.parse({ "--option", "value" }), InvalidConfigurationException);
+    EXPECT_THROW(ProgramDescription{ desc }.parse({ "--option", "-o" }), InvalidParameterException);
   }
 
   {