Fix positional<values<>> options

Author: Sirraide

.gitignore

@@ -1,6 +1,7 @@
 .idea/
 test/out/
 test/tests
+cmake-build-*
 
 # Windows stuff
 .vs/

include/clopts.hh

@@ -811,10 +811,11 @@ private:
         size_t max_vals_opt_name_len{};
         size_t max_len{};
         auto determine_length = [&]<typename opt> {
+            if constexpr (opt::is_values)
+                max_vals_opt_name_len = std::max(max_vals_opt_name_len, opt::name.len);
+
             /// Positional options go on the first line, so ignore them here.
             if constexpr (not detail::is_positional_v<opt>) {
-                if constexpr (opt::is_values)
-                    max_vals_opt_name_len = std::max(max_vals_opt_name_len, opt::name.len);
                 if constexpr (detail::should_print_argument_type<opt>) {
                     auto n = type_name<typename opt::type>();
                     max_len = std::max(max_len, opt::name.len + (n.len + sizeof("<> ") - 1));
@@ -892,7 +893,7 @@ public:
 private:
     /// Handle an option value.
     template <typename opt, bool is_multiple>
-    auto dispatch_option_with_arg(std::string_view opt_str, std::string_view opt_val) {
+    void dispatch_option_with_arg(std::string_view opt_str, std::string_view opt_val) {
         using opt_type = typename opt::type;
 
         /// Mark the option as found.
@@ -1124,16 +1125,16 @@ private:
 
     template <typename opt>
     bool handle_positional_impl(std::string_view opt_str) {
+        static_assert(not detail::is_callback<typename opt::type>, "positional<>s may not have a callback");
+
         /// If we've already encountered this positional option, then return.
         static constexpr bool is_multiple = requires { opt::is_multiple; };
         if constexpr (not is_multiple) {
             if (found<opt::name>()) return false;
         }
 
         /// Otherwise, attempt to parse this as the option value.
-        set_found<opt::name>();
-        if constexpr (is_multiple) ref<opt::name>().push_back(make_arg<typename opt::type>(opt_str));
-        else ref<opt::name>() = make_arg<typename opt::type>(opt_str.data());
+        dispatch_option_with_arg<opt, is_multiple>(opt::name.sv(), opt_str);
         return true;
     }
 

test/CMakeLists.txt

@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.21)
-project(clopts VERSION 2.0.0)
+project(clopts VERSION 2.1.0)
 
 set(CMAKE_CXX_STANDARD 26)
 

test/test.cc

@@ -204,6 +204,75 @@ TEST_CASE("Positional options are required by default") {
     CHECK_THROWS(options::parse(0, nullptr, error_handler));
 }
 
+TEST_CASE("Positional values<> work") {
+    using string_options = clopts<positional<"format", "Output format", values<"foo", "bar">>>;
+    using int_options = clopts<positional<"format", "Output format", values<0, 1>>>;
+
+    SECTION("Correct values are accepted") {
+        std::array args1 = {"test", "foo"};
+        std::array args2 = {"test", "bar"};
+        std::array args3 = {"test", "0"};
+        std::array args4 = {"test", "1"};
+
+        auto opts1 = string_options::parse(args1.size(), args1.data(), error_handler);
+        auto opts2 = string_options::parse(args2.size(), args2.data(), error_handler);
+        auto opts3 = int_options::parse(args3.size(), args3.data(), error_handler);
+        auto opts4 = int_options::parse(args4.size(), args4.data(), error_handler);
+
+        REQUIRE(opts1.get<"format">());
+        REQUIRE(opts2.get<"format">());
+        REQUIRE(opts3.get<"format">());
+        REQUIRE(opts4.get<"format">());
+
+        CHECK(*opts1.get<"format">() == "foo");
+        CHECK(*opts2.get<"format">() == "bar");
+        CHECK(*opts3.get<"format">() == 0);
+        CHECK(*opts4.get<"format">() == 1);
+    }
+
+    SECTION("Invalid values raise an error") {
+        std::array args1 = {"test", "baz"};
+        std::array args2 = {"test", "2"};
+
+        CHECK_THROWS(string_options::parse(args1.size(), args1.data(), error_handler));
+        CHECK_THROWS(int_options::parse(args2.size(), args2.data(), error_handler));
+    }
+}
+
+TEST_CASE("Multiple positional values<> work") {
+    using string_options = clopts<multiple<positional<"format", "Output format", values<"foo", "bar">>>>;
+    using int_options = clopts<multiple<positional<"format", "Output format", values<0, 1>>>>;
+
+    SECTION("Correct values are accepted") {
+        std::array args1 = {"test", "foo", "bar", "foo"};
+        std::array args2 = {"test", "0", "1", "1"};
+
+        auto opts1 = string_options::parse(args1.size(), args1.data(), error_handler);
+        auto opts2 = int_options::parse(args2.size(), args2.data(), error_handler);
+
+        REQUIRE(opts1.get<"format">());
+        REQUIRE(opts2.get<"format">());
+
+        REQUIRE(opts1.get<"format">()->size() == 3);
+        REQUIRE(opts2.get<"format">()->size() == 3);
+
+        CHECK(opts1.get<"format">()->at(0) == "foo");
+        CHECK(opts1.get<"format">()->at(1) == "bar");
+        CHECK(opts1.get<"format">()->at(2) == "foo");
+        CHECK(opts2.get<"format">()->at(0) == 0);
+        CHECK(opts2.get<"format">()->at(1) == 1);
+        CHECK(opts2.get<"format">()->at(2) == 1);
+    }
+
+    SECTION("Invalid values raise an error") {
+        std::array args1 = {"test", "foo", "baz", "foo"};
+        std::array args2 = {"test", "0", "2", "1"};
+
+        CHECK_THROWS(string_options::parse(args1.size(), args1.data(), error_handler));
+        CHECK_THROWS(int_options::parse(args2.size(), args2.data(), error_handler));
+    }
+}
+
 TEST_CASE("Short option options are parsed properly") {
     using options = clopts<
         experimental::short_option<"s", "A string", std::string>,
@@ -360,7 +429,7 @@ TEST_CASE("Calling from main() works as expected") {
 }
 
 TEST_CASE("File option can map a file properly") {
-    auto run = [] <typename file> {
+    auto run = []<typename file> {
         using options = clopts<option<"file", "A file", file>>;
 
         std::array args = {