Skip to content

Commit 99b7441

Browse files
authored
Merge pull request #226 from eseiler/misc/is_option_set
[FIX] is_option_set: match both long and short ids
2 parents d3b3a7b + 6ea2963 commit 99b7441

File tree

5 files changed

+239
-80
lines changed

5 files changed

+239
-80
lines changed

include/sharg/detail/format_parse.hpp

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include <sharg/concept.hpp>
1515
#include <sharg/detail/format_base.hpp>
16+
#include <sharg/detail/id_pair.hpp>
1617

1718
namespace sharg::detail
1819
{
@@ -138,16 +139,6 @@ class format_parse : public format_base
138139
{}
139140
//!\endcond
140141

141-
//!\brief Checks whether `id` is empty.
142-
template <typename id_type>
143-
static bool is_empty_id(id_type const & id)
144-
{
145-
if constexpr (std::same_as<std::remove_cvref_t<id_type>, std::string>)
146-
return id.empty();
147-
else // char
148-
return id == '\0';
149-
}
150-
151142
/*!\brief Finds the position of a short/long identifier in format_parse::arguments.
152143
* \tparam iterator_type The type of iterator that defines the range to search in.
153144
* \tparam id_type The identifier type; must be either of type `char` if it denotes a short identifier or
@@ -169,32 +160,39 @@ class format_parse : public format_base
169160
* The `id` is found by comparing every argument in arguments to `id` prepended with two dashes (`--`)
170161
* or a prefix of such followed by the equal sign `=`.
171162
*/
172-
template <typename iterator_type, typename id_type>
173-
static iterator_type find_option_id(iterator_type begin_it, iterator_type end_it, id_type const & id)
163+
template <typename iterator_type>
164+
static iterator_type find_option_id(iterator_type begin_it, iterator_type end_it, detail::id_pair const & id)
174165
{
175-
if (is_empty_id(id))
166+
bool const short_id_empty{id.empty_short_id()};
167+
bool const long_id_empty{id.empty_long_id()};
168+
169+
if (short_id_empty && long_id_empty)
176170
return end_it;
177171

178-
return (std::find_if(begin_it,
179-
end_it,
180-
[&](std::string const & current_arg)
181-
{
182-
std::string full_id = prepend_dash(id);
183-
184-
if constexpr (std::same_as<id_type, char>) // short id
185-
{
186-
// check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
187-
// "-ovalue", "-o=value", and "-o value".
188-
return current_arg.substr(0, full_id.size()) == full_id;
189-
}
190-
else
191-
{
192-
// only "--opt Value" or "--opt=Value" are valid
193-
return current_arg.substr(0, full_id.size()) == full_id && // prefix is the same
194-
(current_arg.size() == full_id.size()
195-
|| current_arg[full_id.size()] == '='); // space or `=`
196-
}
197-
}));
172+
std::string const short_id = prepend_dash(id.short_id);
173+
std::string const long_id_equals = prepend_dash(id.long_id) + "=";
174+
std::string_view const long_id_space = [&long_id_equals]()
175+
{
176+
std::string_view tmp{long_id_equals};
177+
tmp.remove_suffix(1u);
178+
return tmp;
179+
}();
180+
181+
auto cmp = [&](std::string_view const current_arg)
182+
{
183+
// check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
184+
// "-ovalue", "-o=value", and "-o value".
185+
if (!short_id_empty && current_arg.starts_with(short_id))
186+
return true;
187+
188+
// only "--opt Value" or "--opt=Value" are valid
189+
if (!long_id_empty && (current_arg == long_id_space || current_arg.starts_with(long_id_equals)))
190+
return true;
191+
192+
return false;
193+
};
194+
195+
return std::find_if(begin_it, end_it, cmp);
198196
}
199197

200198
private:

include/sharg/detail/id_pair.hpp

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// SPDX-FileCopyrightText: 2006-2024, Knut Reinert & Freie Universität Berlin
2+
// SPDX-FileCopyrightText: 2016-2024, Knut Reinert & MPI für molekulare Genetik
3+
// SPDX-License-Identifier: BSD-3-Clause
4+
5+
/*!\file
6+
* \author Enrico Seiler <enrico.seiler AT fu-berlin.de>
7+
* \brief Provides sharg::detail::id_pair.
8+
*/
9+
10+
#pragma once
11+
12+
#include <algorithm>
13+
#include <string>
14+
#include <unordered_set>
15+
16+
#include <sharg/platform.hpp>
17+
18+
namespace sharg::detail
19+
{
20+
21+
/*!\brief A simple struct to store a short and a long identifier for an option.
22+
* \ingroup parser
23+
*/
24+
struct id_pair
25+
{
26+
char short_id{}; //!< The short identifier for the option.
27+
std::string long_id{}; //!< The long identifier for the option.
28+
29+
id_pair() = default; //!< Defaulted.
30+
id_pair(id_pair const &) = default; //!< Defaulted.
31+
id_pair & operator=(id_pair const &) = default; //!< Defaulted.
32+
id_pair(id_pair &&) = default; //!< Defaulted.
33+
id_pair & operator=(id_pair &&) = default; //!< Defaulted.
34+
~id_pair() = default; //!< Defaulted.
35+
36+
//!\brief Construct an id_pair from a short ID.
37+
id_pair(char const short_id) : short_id{short_id}
38+
{}
39+
40+
//!\brief Construct an id_pair from a long ID.
41+
id_pair(std::string long_id) : long_id{std::move(long_id)}
42+
{}
43+
44+
//!\brief Construct an id_pair from a short and long ID.
45+
id_pair(char const short_id, std::string long_id) : short_id{short_id}, long_id{std::move(long_id)}
46+
{}
47+
48+
/*!\brief Two id_pairs are equal if their short **or** long ID is equal.
49+
* Empty IDs are not considered for equality. If both IDs are empty, the id_pairs are considered **not** equal.
50+
*/
51+
friend bool operator==(id_pair const & lhs, id_pair const & rhs)
52+
{
53+
return (!lhs.empty_short_id() && lhs.short_id == rhs.short_id)
54+
|| (!lhs.empty_long_id() && lhs.long_id == rhs.long_id);
55+
}
56+
57+
/*!\brief Compares the given short ID with the short ID of the id_pair.
58+
* Returns false if the id_pair's short ID is empty.
59+
*/
60+
friend bool operator==(id_pair const & lhs, char const & rhs)
61+
{
62+
return !lhs.empty_short_id() && lhs.short_id == rhs;
63+
}
64+
65+
/*!\brief Compares the given long ID with the long ID of the id_pair.
66+
* Returns false if the id_pair's long ID is empty.
67+
*/
68+
friend bool operator==(id_pair const & lhs, std::string const & rhs)
69+
{
70+
return !lhs.empty_long_id() && lhs.long_id == rhs;
71+
}
72+
73+
//!\brief Returns true if the short ID is empty.
74+
bool empty_short_id() const noexcept
75+
{
76+
return empty(short_id);
77+
}
78+
79+
//!\brief Returns true if the long ID is empty.
80+
bool empty_long_id() const noexcept
81+
{
82+
return empty(long_id);
83+
}
84+
85+
//!\brief Returns true if both IDs are empty.
86+
bool empty() const noexcept
87+
{
88+
return empty_short_id() && empty_long_id();
89+
}
90+
91+
//!\brief Checks whether id is empty.
92+
template <typename id_type>
93+
static bool empty(id_type const & id) noexcept;
94+
95+
// Note: The following two functions are declared, but not defined.
96+
// We first need to specialise std::hash<id_pair> in the std namespace.
97+
// After that, we can define the functions.
98+
// Defining them here would generate std::hash<id_pair> before the specialisation.
99+
// 1.) Now there are two specialisations for std::hash<id_pair> (error)
100+
// 2.) The default-generated std::hash<id_pair> does actually not work
101+
102+
//!\brief Finds an id_pair in a set of used ids.
103+
template <typename id_type>
104+
static auto find(std::unordered_set<id_pair> const & used_ids, id_type const & id);
105+
106+
//!\brief Checks whether an id is already contained in a set of used ids.
107+
template <typename id_type>
108+
static bool contains(std::unordered_set<id_pair> const & used_ids, id_type const & id);
109+
};
110+
111+
} // namespace sharg::detail
112+
113+
namespace std
114+
{
115+
116+
/*!\brief Hash specialization for sharg::detail::id_pair.
117+
* \ingroup parser
118+
*/
119+
template <>
120+
struct hash<sharg::detail::id_pair>
121+
{
122+
//!\brief Computes the hash value for a given id_pair.
123+
size_t operator()(sharg::detail::id_pair const & value) const noexcept
124+
{
125+
size_t const h1 = std::hash<char>{}(value.short_id);
126+
size_t const h2 = std::hash<std::string>{}(value.long_id);
127+
return h1 ^ (h2 << 1);
128+
}
129+
};
130+
131+
} // namespace std
132+
133+
namespace sharg::detail
134+
{
135+
136+
template <typename id_type>
137+
inline bool id_pair::empty(id_type const & id) noexcept
138+
{
139+
if constexpr (std::same_as<id_type, id_pair>)
140+
return id.empty();
141+
else if constexpr (std::same_as<id_type, char>)
142+
return id == '\0';
143+
else
144+
return id.empty();
145+
}
146+
147+
template <typename id_type>
148+
inline auto id_pair::find(std::unordered_set<id_pair> const & used_ids, id_type const & id)
149+
{
150+
if (empty(id))
151+
return used_ids.end();
152+
153+
return std::ranges::find_if(used_ids,
154+
[&id](id_pair const & pair)
155+
{
156+
return pair == id;
157+
});
158+
}
159+
160+
template <typename id_type>
161+
inline bool id_pair::contains(std::unordered_set<id_pair> const & used_ids, id_type const & id)
162+
{
163+
return find(used_ids, id) != used_ids.end();
164+
}
165+
166+
} // namespace sharg::detail

include/sharg/parser.hpp

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -493,26 +493,22 @@ class parser
493493
if (!parse_was_called)
494494
throw design_error{"You can only ask which options have been set after calling the function `parse()`."};
495495

496-
// the detail::format_parse::find_option_id call in the end expects either a char or std::string
497-
using char_or_string_t = std::conditional_t<std::same_as<id_type, char>, char, std::string>;
498-
char_or_string_t short_or_long_id = {id}; // e.g. convert char * to string here if necessary
496+
detail::id_pair const id_pair{id};
499497

500-
if constexpr (!std::same_as<id_type, char>) // long id was given
498+
if (id_pair.long_id.size() == 1u)
501499
{
502-
if (short_or_long_id.size() == 1)
503-
{
504-
throw design_error{"Long option identifiers must be longer than one character! If " + short_or_long_id
505-
+ "' was meant to be a short identifier, please pass it as a char ('') not a string"
506-
" (\"\")!"};
507-
}
500+
throw design_error{"Long option identifiers must be longer than one character! If " + id_pair.long_id
501+
+ "' was meant to be a short identifier, please pass it as a char ('') not a string"
502+
" (\"\")!"};
508503
}
509504

510-
if (!used_ids.contains(std::string{id}))
505+
auto const it = detail::id_pair::find(used_option_ids, id_pair);
506+
if (it == used_option_ids.end())
511507
throw design_error{"You can only ask for option identifiers that you added with add_option() before."};
512508

513509
// we only need to search for an option before the `option_end_identifier` (`--`)
514-
auto option_end = std::find(format_arguments.begin(), format_arguments.end(), option_end_identifier);
515-
auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), option_end, short_or_long_id);
510+
auto option_end = std::ranges::find(format_arguments, option_end_identifier);
511+
auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), option_end, *it);
516512
return option_it != option_end;
517513
}
518514

@@ -760,8 +756,13 @@ class parser
760756
detail::format_copyright>
761757
format{detail::format_short_help{}};
762758

763-
//!\brief List of option/flag identifiers (excluding -/--) that are already used.
764-
std::unordered_set<std::string> used_ids{"h", "hh", "help", "advanced-help", "export-help", "version", "copyright"};
759+
//!\brief List of option/flag identifiers that are already used.
760+
std::unordered_set<detail::id_pair> used_option_ids{{'h', "help"},
761+
{'\0' /*hh*/, "advanced-help"},
762+
{'\0', "hh"},
763+
{'\0', "export-help"},
764+
{'\0', "version"},
765+
{'\0', "copyright"}};
765766

766767
//!\brief The command line arguments that will be passed to the format.
767768
std::vector<std::string> format_arguments{};
@@ -963,19 +964,6 @@ class parser
963964
format = detail::format_parse(format_arguments);
964965
}
965966

966-
/*!\brief Checks whether the long identifier has already been used before.
967-
* \param[in] id The long identifier of the command line option/flag.
968-
* \returns `true` if an option or flag with the long identifier exists or `false`
969-
* otherwise.
970-
*/
971-
template <typename id_type>
972-
bool id_exists(id_type const & id)
973-
{
974-
if (detail::format_parse::is_empty_id(id))
975-
return false;
976-
return (!(used_ids.insert(std::string({id}))).second);
977-
}
978-
979967
/*!\brief Verifies that the short and the long identifiers are correctly formatted.
980968
* \param[in] short_id The short identifier of the command line option/flag.
981969
* \param[in] long_id The long identifier of the command line option/flag.
@@ -1000,7 +988,7 @@ class parser
1000988
{
1001989
if (short_id == '-' || !is_valid(short_id))
1002990
throw design_error{"Short identifiers may only contain alphanumeric characters, '_', or '@'."};
1003-
if (id_exists(short_id))
991+
if (detail::id_pair::contains(used_option_ids, short_id))
1004992
throw design_error{"Short identifier '" + std::string(1, short_id) + "' was already used before."};
1005993
}
1006994

@@ -1012,9 +1000,11 @@ class parser
10121000
throw design_error{"Long identifiers may not use '-' as first character."};
10131001
if (!std::ranges::all_of(long_id, is_valid))
10141002
throw design_error{"Long identifiers may only contain alphanumeric characters, '_', '-', or '@'."};
1015-
if (id_exists(long_id))
1003+
if (detail::id_pair::contains(used_option_ids, long_id))
10161004
throw design_error{"Long identifier '" + long_id + "' was already used before."};
10171005
}
1006+
1007+
used_option_ids.emplace(short_id, long_id);
10181008
}
10191009

10201010
//!brief Verify the configuration given to a sharg::parser::add_option call.

0 commit comments

Comments
 (0)