From c23759ac6d91d52f9fad0e193a4ad8ddf99e68cf Mon Sep 17 00:00:00 2001 From: Brett Laptop Date: Wed, 19 Feb 2025 13:28:58 -0500 Subject: [PATCH] fix a lot of bugs, current testing works --- CMakeLists.txt | 2 +- include/blt/parse/argparse_v2.h | 71 ++++++++++---- include/blt/std/memory_util.h | 2 +- src/blt/parse/argparse_v2.cpp | 166 +++++++++++++++++++------------- 4 files changed, 153 insertions(+), 88 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 139afa8..e054b89 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.20) include(cmake/color.cmake) -set(BLT_VERSION 4.0.15) +set(BLT_VERSION 4.0.16) set(BLT_TARGET BLT) diff --git a/include/blt/parse/argparse_v2.h b/include/blt/parse/argparse_v2.h index b11b3d0..a49824d 100644 --- a/include/blt/parse/argparse_v2.h +++ b/include/blt/parse/argparse_v2.h @@ -76,8 +76,12 @@ namespace blt::argparse explicit bad_flag(const std::string& message): std::runtime_error(message) { } + }; - explicit bad_flag(const char* message): std::runtime_error(message) + class bad_positional final : public std::runtime_error + { + public: + explicit bad_positional(const std::string& message): std::runtime_error(message) { } }; @@ -114,6 +118,14 @@ namespace blt::argparse } }; + class bad_choice_error final : public std::runtime_error + { + public: + explicit bad_choice_error(const std::string& message): std::runtime_error(message) + { + } + }; + class subparse_error final : public std::exception { public: @@ -137,10 +149,12 @@ namespace blt::argparse [[nodiscard]] const char* what() const noexcept override { - return "Please use error_string() method instead of what(). This exception should *always* be caught!"; + m_error_string = error_string(); + return m_error_string.c_str(); } private: + mutable std::string m_error_string; std::string_view m_found_string; std::vector> m_allowed_strings; }; @@ -372,7 +386,7 @@ namespace blt::argparse m_data.insert(value); } - hashmap_t m_data; + hashmap_t m_data; }; class argument_builder_t @@ -384,11 +398,11 @@ namespace blt::argparse { m_dest_func = [](const std::string_view dest, argument_storage_t& storage, std::string_view value) { - storage.m_data.insert({dest, value}); + storage.m_data.emplace(std::string{dest}, value); }; m_dest_vec_func = [](const std::string_view dest, argument_storage_t& storage, const std::vector& values) { - storage.m_data.insert({dest, values}); + storage.m_data.emplace(std::string{dest}, values); }; } @@ -398,7 +412,7 @@ namespace blt::argparse static_assert(detail::arg_data_helper_t::template is_type_stored_v, "Type is not valid to be stored/converted as an argument"); m_dest_func = [](const std::string_view dest, argument_storage_t& storage, std::string_view value) { - storage.m_data.insert({dest, detail::arg_string_converter_t::convert(value)}); + storage.m_data.emplace(std::string{dest}, detail::arg_string_converter_t::convert(value)); }; m_dest_vec_func = [](const std::string_view dest, argument_storage_t& storage, const std::vector& values) { @@ -410,7 +424,7 @@ namespace blt::argparse throw detail::type_error("Invalid type conversion. Trying to add type " + blt::type_string() + " but this does not match existing type index '" + std::to_string(data.index()) + "'!"); } - std::vector& converted_values = std::get>(data); + auto& converted_values = std::get>(data); for (const auto& value : values) converted_values.push_back(detail::arg_string_converter_t::convert(value)); } @@ -419,7 +433,7 @@ namespace blt::argparse std::vector converted_values; for (const auto& value : values) converted_values.push_back(detail::arg_string_converter_t::convert(value)); - storage.m_data.insert({dest, converted_values}); + storage.m_data.emplace(std::string{dest}, std::move(converted_values)); } }; return *this; @@ -440,6 +454,13 @@ namespace blt::argparse as_type(); set_default(true); break; + case action_t::STORE_CONST: + set_nargs(0); + break; + case action_t::COUNT: + set_nargs(0); + as_type(); + break; default: break; } @@ -471,6 +492,14 @@ namespace blt::argparse } argument_builder_t& set_choices(const std::vector& choices) + { + m_choices = hashset_t{}; + for (const auto& choice : choices) + m_choices->emplace(choice); + return *this; + } + + argument_builder_t& set_choices(const hashset_t& choices) { m_choices = choices; return *this; @@ -500,7 +529,7 @@ namespace blt::argparse nargs_v m_nargs = 1; // number of arguments to consume std::optional m_metavar; // variable name to be used in the help string std::optional m_help; // help string to be used in the help string - std::optional> m_choices; // optional allowed choices for this argument + std::optional> m_choices; // optional allowed choices for this argument std::optional m_default_value; std::optional m_const_value; std::optional m_dest; @@ -525,20 +554,20 @@ namespace blt::argparse argument_builder_t& add_flag(const std::string_view arg, Aliases... aliases) { static_assert( - std::conjunction_v, std::is_constructible< - std::string_view, Aliases>>...>, + std::conjunction_v, std::is_constructible< + std::string, Aliases>>...>, "Arguments must be of type string_view, convertible to string_view or be string_view constructable"); - m_argument_builders.emplace_back(); - m_flag_arguments.insert({arg, &m_argument_builders.back()}); - (m_flag_arguments.insert({std::string_view{aliases}, &m_argument_builders.back()}), ...); - return m_argument_builders.back(); + m_argument_builders.emplace_back(std::make_unique()); + m_flag_arguments.emplace(arg, m_argument_builders.back().get()); + (m_flag_arguments.emplace(aliases, m_argument_builders.back().get()), ...); + return *m_argument_builders.back().get(); } argument_builder_t& add_positional(const std::string_view arg) { - m_argument_builders.emplace_back(); - m_positional_arguments.insert({arg, &m_argument_builders.back()}); - return m_argument_builders.back(); + m_argument_builders.emplace_back(std::make_unique()); + m_positional_arguments.emplace(arg, m_argument_builders.back().get()); + return *m_argument_builders.back(); } argument_subparser_t& add_subparser(std::string_view dest); @@ -609,10 +638,12 @@ namespace blt::argparse } private: + void handle_compound_flags(hashset_t& found_flags, argument_storage_t& parsed_args, argument_consumer_t& consumer, + const argument_string_t& arg); void parse_flag(argument_storage_t& parsed_args, argument_consumer_t& consumer, std::string_view arg); void parse_positional(argument_storage_t& parsed_args, argument_consumer_t& consumer, std::string_view arg); static void handle_missing_and_default_args(hashmap_t& arguments, - const hashset_t& found, + const hashset_t& found, argument_storage_t& parsed_args, std::string_view type); std::optional m_name; @@ -620,7 +651,7 @@ namespace blt::argparse std::optional m_description; std::optional m_epilogue; std::vector> m_subparsers; - std::vector m_argument_builders; + std::vector> m_argument_builders; hashmap_t m_flag_arguments; hashmap_t m_positional_arguments; hashset_t allowed_flag_prefixes = {'-', '+', '/'}; diff --git a/include/blt/std/memory_util.h b/include/blt/std/memory_util.h index 2c73912..c2041a4 100644 --- a/include/blt/std/memory_util.h +++ b/include/blt/std/memory_util.h @@ -134,7 +134,7 @@ namespace blt::mem return fromBytes(in, *out); } - static std::size_t next_byte_allocation(std::size_t prev_size, std::size_t default_allocation_block = 8192, std::size_t default_size = 16) + inline std::size_t next_byte_allocation(std::size_t prev_size, std::size_t default_allocation_block = 8192, std::size_t default_size = 16) { if (prev_size < default_size) return default_size; diff --git a/src/blt/parse/argparse_v2.cpp b/src/blt/parse/argparse_v2.cpp index d6b7f8e..28c56a3 100644 --- a/src/blt/parse/argparse_v2.cpp +++ b/src/blt/parse/argparse_v2.cpp @@ -23,6 +23,25 @@ namespace blt::argparse { + constexpr static auto printer_primitive = [](const auto& v) + { + std::cout << v; + }; + + constexpr static auto printer_vector = [](const auto& v) + { + std::cout << "["; + for (const auto& [i, a] : enumerate(v)) + { + std::cout << a; + if (i != v.size() - 1) + std::cout << ", "; + } + std::cout << "]"; + }; + + auto print_visitor = detail::arg_meta_type_helper_t::make_visitor(printer_primitive, printer_vector); + template size_t get_const_char_size(const T& t) { @@ -44,7 +63,7 @@ namespace blt::argparse } } - template + template auto ensure_is_string(T&& t) { if constexpr (std::is_arithmetic_v>) @@ -211,7 +230,7 @@ namespace blt::argparse parser.parse(args); BLT_ASSERT(false && "Parsing should fail with invalid flag prefix '!'"); } - catch (const bad_flag& _) + catch (...) { BLT_ASSERT(true && "Correctly threw on bad flag prefix"); } @@ -225,7 +244,7 @@ namespace blt::argparse const std::vector args = {"-vvv"}; const auto parsed_args = parser.parse(args); - BLT_ASSERT(parsed_args.get("-v") == 3 && "Flag '-v' should count occurrences in compound form"); + BLT_ASSERT(parsed_args.get("-v") == 3 && "Flag '-v' should count occurrences in compound form"); } void test_combination_of_valid_and_invalid_flags() @@ -242,7 +261,7 @@ namespace blt::argparse parser.parse(args); BLT_ASSERT(false && "Parsing should fail due to invalid flag '!z'"); } - catch (const bad_flag& _) + catch (...) { BLT_ASSERT(true && "Correctly threw an exception for invalid flag"); } @@ -318,61 +337,31 @@ namespace blt::argparse argument_storage_t argument_parser_t::parse(argument_consumer_t& consumer) { - hashset_t found_flags; - hashset_t found_positional; + hashset_t found_flags; + hashset_t found_positional; argument_storage_t parsed_args; // first, we consume flags which may be part of this parser while (consumer.can_consume() && consumer.peek().is_flag()) + handle_compound_flags(found_flags, parsed_args, consumer, consumer.consume()); + + for (auto& [key, subparser] : m_subparsers) { - const auto key = consumer.consume(); - const auto flag = m_flag_arguments.find(key.get_argument()); - if (flag == m_flag_arguments.end()) - throw detail::bad_flag(make_string("Error: Unknown flag: ", key.get_argument())); - found_flags.insert(key.get_argument()); - parse_flag(parsed_args, consumer, key.get_argument()); - } - try - { - for (auto& [key, subparser] : m_subparsers) - { - auto [parsed_subparser, storage] = subparser.parse(consumer); - storage.m_data.insert({key, detail::arg_data_t{parsed_subparser.get_argument()}}); - parsed_args.add(storage); - } - } - catch (const detail::missing_argument_error& e) - { - std::cerr << "Error: " << e.what() << std::endl; - print_usage(); - exit(1); - } catch (const detail::subparse_error& e) - { - std::cerr << e.error_string() << std::endl; - exit(1); + auto [parsed_subparser, storage] = subparser.parse(consumer); + storage.m_data.emplace(std::string{key}, detail::arg_data_t{parsed_subparser.get_argument()}); + parsed_args.add(storage); } + while (consumer.can_consume()) { const auto key = consumer.consume(); if (key.is_flag()) - { - const auto flag = m_flag_arguments.find(key.get_argument()); - if (flag == m_flag_arguments.end()) - { - std::cerr << "Error: Unknown flag: " << key.get_argument() << std::endl; - exit(1); - } - found_flags.insert(key.get_argument()); - parse_flag(parsed_args, consumer, key.get_argument()); - } + handle_compound_flags(found_flags, parsed_args, consumer, key); else { const auto pos = m_positional_arguments.find(key.get_argument()); if (pos == m_positional_arguments.end()) - { - std::cerr << "Error: Unknown positional argument: " << key.get_argument() << std::endl; - exit(1); - } - found_positional.insert(key.get_argument()); + throw detail::bad_positional(make_string("Error: Unknown positional argument: ", key.get_argument())); + found_positional.insert(std::string{key.get_argument()}); parse_positional(parsed_args, consumer, key.get_argument()); } } @@ -394,10 +383,37 @@ namespace blt::argparse { } + void argument_parser_t::handle_compound_flags(hashset_t& found_flags, argument_storage_t& parsed_args, + argument_consumer_t& consumer, const argument_string_t& arg) + { + // i kinda hate this, TODO? + std::vector compound_flags; + if (arg.get_flag().size() == 1) + { + for (const auto c : arg.get_name()) + compound_flags.emplace_back(std::string{arg.get_flag()} + c); + } + else + { + if (arg.get_flag().size() > 2) + throw detail::bad_flag(make_string("Error: Flag '", arg.get_argument(), "' is too long!")); + compound_flags.emplace_back(arg.get_argument()); + } + + for (const auto& flag_key : compound_flags) + { + const auto flag = m_flag_arguments.find(flag_key); + if (flag == m_flag_arguments.end()) + throw detail::bad_flag(make_string("Error: Unknown flag: ", flag_key)); + found_flags.insert(flag_key); + parse_flag(parsed_args, consumer, flag_key); + } + } + void argument_parser_t::parse_flag(argument_storage_t& parsed_args, argument_consumer_t& consumer, const std::string_view arg) { - auto flag = m_flag_arguments[arg]; - auto dest = flag->m_dest.value_or(std::string{arg}); + auto flag = m_flag_arguments.find(arg)->second; + const auto dest = flag->m_dest.value_or(std::string{arg}); std::visit(lambda_visitor{ [&parsed_args, &consumer, &dest, &flag, arg](const nargs_t arg_enum) { @@ -444,6 +460,26 @@ namespace blt::argparse } args.push_back(consumer.consume().get_argument()); } + if (flag->m_choices) + { + auto& choices = *flag->m_choices; + for (const auto str : args) + { + if (!choices.contains(str)) + { + std::string valid_choices = "{"; + for (const auto& [i, choice] : enumerate(choices)) + { + valid_choices += choice; + if (i != choices.size() - 1) + valid_choices += ", "; + } + valid_choices += "}"; + throw detail::bad_choice_error(make_string('\'', str, "' is not a valid choice for argument '", arg, + "'! Expected one of ", valid_choices)); + } + } + } if (args.size() != static_cast(argc)) { throw std::runtime_error( @@ -452,8 +488,6 @@ namespace blt::argparse "Please report as an issue on the GitHub"); } - BLT_TRACE("Running action %d on dest %s", static_cast(flag->m_action), dest.c_str()); - switch (flag->m_action) { case action_t::STORE: @@ -508,7 +542,7 @@ namespace blt::argparse else { auto visitor = detail::arg_meta_type_helper_t::make_visitor( - [&flag, &parsed_args, &dest](auto& primitive) + [&parsed_args, &dest](auto& primitive) { std::vector> vec; vec.push_back(primitive); @@ -525,11 +559,13 @@ namespace blt::argparse if (argc != 0) { print_usage(); - throw detail::unexpected_argument_error("Store const flag called with an argument."); + throw detail::unexpected_argument_error( + make_string("Argument '", arg, "' is store const but called with an argument.")); } if (!flag->m_const_value) - throw detail::missing_value_error("Store const flag called with no value. "); - parsed_args.m_data.insert({dest, *flag->m_const_value}); + throw detail::missing_value_error( + make_string("Argument '", arg, "' is store const, but const storage has no value.")); + parsed_args.m_data.emplace(dest, *flag->m_const_value); break; case action_t::STORE_TRUE: if (argc != 0) @@ -537,7 +573,7 @@ namespace blt::argparse print_usage(); throw detail::unexpected_argument_error("Store true flag called with an argument."); } - parsed_args.m_data.insert({dest, true}); + parsed_args.m_data.emplace(dest, true); break; case action_t::STORE_FALSE: if (argc != 0) @@ -551,12 +587,12 @@ namespace blt::argparse if (parsed_args.m_data.contains(dest)) { auto visitor = detail::arg_meta_type_helper_t::make_visitor( - [&args](auto& primitive) -> detail::arg_data_t + [](auto& primitive) -> detail::arg_data_t { using type = meta::remove_cvref_t; - if constexpr (std::is_convertible_v) + if constexpr (std::is_convertible_v) { - return primitive + static_cast(args.size()); + return primitive + static_cast(1); } else throw detail::type_error("Error: count called but stored type is " + blt::type_string()); @@ -569,8 +605,8 @@ namespace blt::argparse ); parsed_args.m_data[dest] = std::visit(visitor, parsed_args.m_data[dest]); } - else - parsed_args.m_data.insert({dest, args.size()}); + else // I also hate this! + flag->m_dest_func(dest, parsed_args, "1"); break; case action_t::HELP: print_help(); @@ -588,7 +624,7 @@ namespace blt::argparse } void argument_parser_t::handle_missing_and_default_args(hashmap_t& arguments, - const hashset_t& found, argument_storage_t& parsed_args, + const hashset_t& found, argument_storage_t& parsed_args, const std::string_view type) { for (const auto& [key, value] : arguments) @@ -596,13 +632,11 @@ namespace blt::argparse if (!found.contains(key)) { if (value->m_required) - { - std::cerr << "Error: " << type << " argument '" << key << "' was not found but is required by the program" << std::endl; - exit(1); - } + throw detail::missing_argument_error(make_string("Error: ", type, " argument '", key, + "' was not found but is required by the program")); auto dest = value->m_dest.value_or(std::string{key}); if (value->m_default_value && !parsed_args.contains(dest)) - parsed_args.m_data.insert({dest, *value->m_default_value}); + parsed_args.m_data.emplace(dest, *value->m_default_value); } } }