Skip to content

Commit 13800ef

Browse files
committed
maybe better?
1 parent e58a7f4 commit 13800ef

File tree

4 files changed

+103
-79
lines changed

4 files changed

+103
-79
lines changed

include/sharg/detail/format_parse.hpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,27 +173,33 @@ class format_parse : public format_base
173173
template <typename iterator_type>
174174
static iterator_type find_option_id(iterator_type begin_it, iterator_type end_it, detail::id_pair const & id)
175175
{
176-
bool const short_id_empty{id.short_id.empty()};
177-
bool const long_id_empty{id.long_id.empty()};
176+
bool const short_id_empty{id.empty_short_id()};
177+
bool const long_id_empty{id.empty_long_id()};
178178

179179
if (short_id_empty && long_id_empty)
180180
return end_it;
181181

182-
auto cmp = [&](std::string const & current_arg)
182+
std::string const short_id = prepend_dash(id.short_id);
183+
std::string const long_id_equals = prepend_dash(id.long_id) + "=";
184+
std::string_view const long_id_space = [&long_id_equals]()
185+
{
186+
std::string_view tmp{long_id_equals};
187+
tmp.remove_suffix(1u);
188+
return tmp;
189+
}();
190+
191+
auto cmp = [&](std::string_view const current_arg)
183192
{
184193
// check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
185194
// "-ovalue", "-o=value", and "-o value".
186-
bool const short_id_found = !short_id_empty && current_arg.starts_with("-")
187-
&& current_arg.substr(1, id.short_id.size()) == id.short_id;
195+
if (!short_id_empty && current_arg.starts_with(short_id))
196+
return true;
188197

189198
// only "--opt Value" or "--opt=Value" are valid
190-
bool const long_id_found = !short_id_found // Don't need to check long_id if short_id was found
191-
&& !long_id_empty && current_arg.starts_with("--")
192-
&& current_arg.substr(2, id.long_id.size()) == id.long_id // prefix is the same
193-
&& (current_arg.size() == id.long_id.size() + 2
194-
|| current_arg[id.long_id.size() + 2] == '='); // space or `=`
199+
if (!long_id_empty && (current_arg == long_id_space || current_arg.starts_with(long_id_equals)))
200+
return true;
195201

196-
return short_id_found || long_id_found;
202+
return false;
197203
};
198204

199205
return std::ranges::find_if(begin_it, end_it, cmp);

include/sharg/detail/id_pair.hpp

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include <string>
1313
#include <unordered_set>
1414

15+
#include <sharg/platform.hpp>
16+
1517
//!\cond
1618

1719
namespace sharg::detail
@@ -22,40 +24,65 @@ namespace sharg::detail
2224
*/
2325
struct id_pair
2426
{
25-
std::string short_id{}; //!< The short identifier for the option.
26-
std::string long_id{}; //!< The long identifier for the option.
27+
char short_id{}; //!< The short identifier for the option.
28+
std::string long_id{}; //!< The long identifier for the option.
2729

2830
id_pair() = default;
2931
id_pair(id_pair const &) = default;
30-
id_pair(id_pair &&) = default;
3132
id_pair & operator=(id_pair const &) = default;
33+
id_pair(id_pair &&) = default;
3234
id_pair & operator=(id_pair &&) = default;
3335
~id_pair() = default;
3436

35-
id_pair(std::string id_short, std::string id_long) : short_id{std::move(id_short)}, long_id{std::move(id_long)}
37+
id_pair(char const short_id) : short_id{short_id}
3638
{}
3739

38-
id_pair(char const id_short, std::string id_long) :
39-
short_id{std::string(id_short != '\0', id_short)},
40-
long_id{std::move(id_long)}
40+
id_pair(std::string long_id) : long_id{std::move(long_id)}
4141
{}
4242

43-
id_pair(char const id_short) : short_id{std::string(id_short != '\0', id_short)}
44-
{}
45-
46-
id_pair(std::string id_long) : long_id{std::move(id_long)}
43+
id_pair(char const short_id, std::string long_id) : short_id{short_id}, long_id{std::move(long_id)}
4744
{}
4845

4946
//!\brief Compares two id_pairs for equality.
50-
friend auto operator<=>(id_pair const &, id_pair const &) = default;
47+
friend bool operator==(id_pair const & lhs, id_pair const & rhs)
48+
{
49+
return (!lhs.empty_short_id() && lhs.short_id == rhs.short_id)
50+
|| (!lhs.empty_long_id() && lhs.long_id == rhs.long_id);
51+
}
52+
53+
friend bool operator==(id_pair const & lhs, char const & rhs)
54+
{
55+
return !lhs.empty_short_id() && lhs.short_id == rhs;
56+
}
57+
58+
friend bool operator==(id_pair const & lhs, std::string const & rhs)
59+
{
60+
return !lhs.empty_long_id() && lhs.long_id == rhs;
61+
}
62+
63+
bool empty_short_id() const
64+
{
65+
return empty(short_id);
66+
}
67+
68+
bool empty_long_id() const
69+
{
70+
return empty(long_id);
71+
}
5172

52-
static auto find(std::unordered_set<id_pair> const & used_ids, char const short_id);
73+
bool empty() const
74+
{
75+
return empty_short_id() && empty_long_id();
76+
}
5377

54-
static auto find(std::unordered_set<id_pair> const & used_ids, std::string const & long_id);
78+
template <typename id_type>
79+
static bool empty(id_type const & id);
5580

56-
static bool contains(std::unordered_set<id_pair> const & used_ids, char const short_id);
81+
template <typename id_type>
82+
static auto find(std::unordered_set<id_pair> const & used_ids, id_type const & id);
5783

58-
static bool contains(std::unordered_set<id_pair> const & used_ids, std::string const & long_id);
84+
template <typename id_type>
85+
static bool contains(std::unordered_set<id_pair> const & used_ids, id_type const & id);
5986
};
6087

6188
} // namespace sharg::detail
@@ -68,8 +95,8 @@ struct hash<sharg::detail::id_pair>
6895
{
6996
size_t operator()(sharg::detail::id_pair const & value) const noexcept
7097
{
71-
size_t h1 = std::hash<std::string>{}(value.short_id);
72-
size_t h2 = std::hash<std::string>{}(value.long_id);
98+
size_t const h1 = std::hash<char>{}(value.short_id);
99+
size_t const h2 = std::hash<std::string>{}(value.long_id);
73100
return h1 ^ (h2 << 1);
74101
}
75102
};
@@ -79,35 +106,34 @@ struct hash<sharg::detail::id_pair>
79106
namespace sharg::detail
80107
{
81108

82-
inline auto id_pair::find(std::unordered_set<id_pair> const & used_ids, char const short_id)
83-
{
84-
std::string const id_str(1, short_id);
85-
auto it = std::ranges::find_if(used_ids,
86-
[&id_str](id_pair const & id)
87-
{
88-
return id.short_id == id_str;
89-
});
90-
return it;
91-
}
92-
93-
inline auto id_pair::find(std::unordered_set<id_pair> const & used_ids, std::string const & long_id)
109+
template <typename id_type>
110+
inline bool id_pair::empty(id_type const & id)
94111
{
95-
auto it = std::ranges::find_if(used_ids,
96-
[&long_id](id_pair const & id)
97-
{
98-
return id.long_id == long_id;
99-
});
100-
return it;
112+
if constexpr (std::same_as<id_type, id_pair>)
113+
return id.empty();
114+
else if constexpr (std::same_as<id_type, char>)
115+
return id == '\0';
116+
else
117+
return id.empty();
101118
}
102119

103-
inline bool id_pair::contains(std::unordered_set<id_pair> const & used_ids, char const short_id)
120+
template <typename id_type>
121+
inline auto id_pair::find(std::unordered_set<id_pair> const & used_ids, id_type const & id)
104122
{
105-
return id_pair::find(used_ids, short_id) != used_ids.end();
123+
if (empty(id))
124+
return used_ids.end();
125+
126+
return std::ranges::find_if(used_ids,
127+
[&id](id_pair const & pair)
128+
{
129+
return pair == id;
130+
});
106131
}
107132

108-
inline bool id_pair::contains(std::unordered_set<id_pair> const & used_ids, std::string const & long_id)
133+
template <typename id_type>
134+
inline bool id_pair::contains(std::unordered_set<id_pair> const & used_ids, id_type const & id)
109135
{
110-
return id_pair::find(used_ids, long_id) != used_ids.end();
136+
return find(used_ids, id) != used_ids.end();
111137
}
112138

113139
} // namespace sharg::detail

include/sharg/parser.hpp

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -495,34 +495,23 @@ class parser
495495
if (!parse_was_called)
496496
throw design_error{"You can only ask which options have been set after calling the function `parse()`."};
497497

498-
constexpr bool id_is_long = !std::same_as<id_type, char>;
498+
detail::id_pair const id_pair{id};
499499

500-
// the detail::format_parse::find_option_id call in the end expects either a char or std::string
501-
using char_or_string_t = std::conditional_t<id_is_long, std::string, char>;
502-
char_or_string_t const short_or_long_id{id}; // e.g. convert char * to string here if necessary
503-
504-
if constexpr (id_is_long) // long id was given
500+
if (id_pair.long_id.size() == 1u)
505501
{
506-
if (short_or_long_id.size() == 1)
507-
{
508-
throw design_error{"Long option identifiers must be longer than one character! If " + short_or_long_id
509-
+ "' was meant to be a short identifier, please pass it as a char ('') not a string"
510-
" (\"\")!"};
511-
}
502+
throw design_error{"Long option identifiers must be longer than one character! If " + id_pair.long_id
503+
+ "' was meant to be a short identifier, please pass it as a char ('') not a string"
504+
" (\"\")!"};
512505
}
513506

514-
detail::id_pair const & ids = [this, &short_or_long_id]()
515-
{
516-
auto it = detail::id_pair::find(used_option_ids, short_or_long_id);
517-
if (it == used_option_ids.end())
518-
throw design_error{"You can only ask for option identifiers that you added with add_option() before."};
519-
return *it;
520-
}();
507+
auto const it = detail::id_pair::find(used_option_ids, id_pair);
508+
if (it == used_option_ids.end())
509+
throw design_error{"You can only ask for option identifiers that you added with add_option() before."};
521510

522511
// we only need to search for an option before the `option_end_identifier` (`--`)
523-
auto end_of_options = std::find(format_arguments.begin(), format_arguments.end(), option_end_identifier);
524-
auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), end_of_options, ids);
525-
return option_it != end_of_options;
512+
auto option_end = std::ranges::find(format_arguments, option_end_identifier);
513+
auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), option_end, *it);
514+
return option_it != option_end;
526515
}
527516

528517
//!\name Structuring the Help Page
@@ -749,11 +738,11 @@ class parser
749738
format{detail::format_short_help{}};
750739

751740
//!\brief List of option/flag identifiers that are already used.
752-
std::unordered_set<detail::id_pair> used_option_ids{{"h", "help"},
753-
{"hh", "advanced-help"},
754-
{"", "export-help"},
755-
{"", "version"},
756-
{"", "copyright"}};
741+
std::unordered_set<detail::id_pair> used_option_ids{{'h', "help"},
742+
{'\0' /*hh*/, "advanced-help"},
743+
{'\0', "export-help"},
744+
{'\0', "version"},
745+
{'\0', "copyright"}};
757746

758747
//!\brief The command line arguments that will be passed to the format.
759748
std::vector<std::string> format_arguments{};

test/unit/parser/format_parse_test.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,9 +758,12 @@ TEST_F(format_parse_test, is_option_set)
758758
EXPECT_FALSE(parser.is_option_set("bar")); // --bar is behind the `--`
759759

760760
// errors:
761+
size_t counter{};
762+
761763
auto expect_design_error = [&](auto && option)
762764
{
763-
EXPECT_THROW(parser.is_option_set(option), sharg::design_error);
765+
EXPECT_THROW(parser.is_option_set(option), sharg::design_error) << counter;
766+
++counter;
764767
};
765768

766769
expect_design_error("l"); // short identifiers are passed as chars not strings

0 commit comments

Comments
 (0)