[FIX] is_option_set: match both long and short ids

include/sharg/detail/format_parse.hpp

@@ -13,6 +13,7 @@
 
 #include <sharg/concept.hpp>
 #include <sharg/detail/format_base.hpp>
+#include <sharg/detail/id_pair.hpp>
 
 namespace sharg::detail
 {
@@ -138,16 +139,6 @@ class format_parse : public format_base
     {}
     //!\endcond
 
-    //!\brief Checks whether `id` is empty.
-    template <typename id_type>
-    static bool is_empty_id(id_type const & id)
-    {
-        if constexpr (std::same_as<std::remove_cvref_t<id_type>, std::string>)
-            return id.empty();
-        else // char
-            return id == '\0';
-    }
-
     /*!\brief Finds the position of a short/long identifier in format_parse::arguments.
      * \tparam iterator_type The type of iterator that defines the range to search in.
      * \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
      * The `id` is found by comparing every argument in arguments to `id` prepended with two dashes (`--`)
      * or a prefix of such followed by the equal sign `=`.
      */
-    template <typename iterator_type, typename id_type>
-    static iterator_type find_option_id(iterator_type begin_it, iterator_type end_it, id_type const & id)
+    template <typename iterator_type>
+    static iterator_type find_option_id(iterator_type begin_it, iterator_type end_it, detail::id_pair const & id)
     {
-        if (is_empty_id(id))
+        bool const short_id_empty{id.empty_short_id()};
+        bool const long_id_empty{id.empty_long_id()};
+
+        if (short_id_empty && long_id_empty)
             return end_it;
 
-        return (std::find_if(begin_it,
-                             end_it,
-                             [&](std::string const & current_arg)
-                             {
-                                 std::string full_id = prepend_dash(id);
-
-                                 if constexpr (std::same_as<id_type, char>) // short id
-                                 {
-                                     // check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
-                                     // "-ovalue", "-o=value", and "-o value".
-                                     return current_arg.substr(0, full_id.size()) == full_id;
-                                 }
-                                 else
-                                 {
-                                     // only "--opt Value" or "--opt=Value" are valid
-                                     return current_arg.substr(0, full_id.size()) == full_id && // prefix is the same
-                                            (current_arg.size() == full_id.size()
-                                             || current_arg[full_id.size()] == '='); // space or `=`
-                                 }
-                             }));
+        std::string const short_id = prepend_dash(id.short_id);
+        std::string const long_id_equals = prepend_dash(id.long_id) + "=";
+        std::string_view const long_id_space = [&long_id_equals]()
+        {
+            std::string_view tmp{long_id_equals};
+            tmp.remove_suffix(1u);
+            return tmp;
+        }();
+
+        auto cmp = [&](std::string_view const current_arg)
+        {
+            // check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
+            // "-ovalue", "-o=value", and "-o value".
+            if (!short_id_empty && current_arg.starts_with(short_id))
+                return true;
+
+            // only "--opt Value" or "--opt=Value" are valid
+            if (!long_id_empty && (current_arg == long_id_space || current_arg.starts_with(long_id_equals)))
+                return true;
+
+            return false;
+        };
+
+        return std::find_if(begin_it, end_it, cmp);
     }
 
 private:

include/sharg/detail/id_pair.hpp

@@ -0,0 +1,166 @@
+// SPDX-FileCopyrightText: 2006-2024, Knut Reinert & Freie Universität Berlin
+// SPDX-FileCopyrightText: 2016-2024, Knut Reinert & MPI für molekulare Genetik
+// SPDX-License-Identifier: BSD-3-Clause
+
+/*!\file
+ * \author Enrico Seiler <enrico.seiler AT fu-berlin.de>
+ * \brief Provides sharg::detail::id_pair.
+ */
+
+#pragma once
+
+#include <algorithm>
+#include <string>
+#include <unordered_set>
+
+#include <sharg/platform.hpp>
+
+namespace sharg::detail
+{
+
+/*!\brief A simple struct to store a short and a long identifier for an option.
+ * \ingroup parser
+ */
+struct id_pair
+{
+    char short_id{};       //!< The short identifier for the option.
+    std::string long_id{}; //!< The long identifier for the option.
+
+    id_pair() = default;                            //!< Defaulted.
+    id_pair(id_pair const &) = default;             //!< Defaulted.
+    id_pair & operator=(id_pair const &) = default; //!< Defaulted.
+    id_pair(id_pair &&) = default;                  //!< Defaulted.
+    id_pair & operator=(id_pair &&) = default;      //!< Defaulted.
+    ~id_pair() = default;                           //!< Defaulted.
+
+    //!\brief Construct an id_pair from a short ID.
+    id_pair(char const short_id) : short_id{short_id}
+    {}
+
+    //!\brief Construct an id_pair from a long ID.
+    id_pair(std::string long_id) : long_id{std::move(long_id)}
+    {}
+
+    //!\brief Construct an id_pair from a short and long ID.
+    id_pair(char const short_id, std::string long_id) : short_id{short_id}, long_id{std::move(long_id)}
+    {}
+
+    /*!\brief Two id_pairs are equal if their short **or** long ID is equal.
+     * Empty IDs are not considered for equality. If both IDs are empty, the id_pairs are considered **not** equal.
+     */
+    friend bool operator==(id_pair const & lhs, id_pair const & rhs)
+    {
+        return (!lhs.empty_short_id() && lhs.short_id == rhs.short_id)
+            || (!lhs.empty_long_id() && lhs.long_id == rhs.long_id);
+    }
+
+    /*!\brief Compares the given short ID with the short ID of the id_pair.
+     * Returns false if the id_pair's short ID is empty.
+     */
+    friend bool operator==(id_pair const & lhs, char const & rhs)
+    {
+        return !lhs.empty_short_id() && lhs.short_id == rhs;
+    }
+
+    /*!\brief Compares the given long ID with the long ID of the id_pair.
+     * Returns false if the id_pair's long ID is empty.
+     */
+    friend bool operator==(id_pair const & lhs, std::string const & rhs)
+    {
+        return !lhs.empty_long_id() && lhs.long_id == rhs;
+    }
+
+    //!\brief Returns true if the short ID is empty.
+    bool empty_short_id() const noexcept
+    {
+        return empty(short_id);
+    }
+
+    //!\brief Returns true if the long ID is empty.
+    bool empty_long_id() const noexcept
+    {
+        return empty(long_id);
+    }
+
+    //!\brief Returns true if both IDs are empty.
+    bool empty() const noexcept
+    {
+        return empty_short_id() && empty_long_id();
+    }
+
+    //!\brief Checks whether id is empty.
+    template <typename id_type>
+    static bool empty(id_type const & id) noexcept;
+
+    // Note: The following two functions are declared, but not defined.
+    // We first need to specialise std::hash<id_pair> in the std namespace.
+    // After that, we can define the functions.
+    // Defining them here would generate std::hash<id_pair> before the specialisation.
+    // 1.) Now there are two specialisations for std::hash<id_pair> (error)
+    // 2.) The default-generated std::hash<id_pair> does actually not work
+
+    //!\brief Finds an id_pair in a set of used ids.
+    template <typename id_type>
+    static auto find(std::unordered_set<id_pair> const & used_ids, id_type const & id);
+
+    //!\brief Checks whether an id is already contained in a set of used ids.
+    template <typename id_type>
+    static bool contains(std::unordered_set<id_pair> const & used_ids, id_type const & id);
+};
+
+} // namespace sharg::detail
+
+namespace std
+{
+
+/*!\brief Hash specialization for sharg::detail::id_pair.
+ * \ingroup parser
+ */
+template <>
+struct hash<sharg::detail::id_pair>
+{
+    //!\brief Computes the hash value for a given id_pair.
+    size_t operator()(sharg::detail::id_pair const & value) const noexcept
+    {
+        size_t const h1 = std::hash<char>{}(value.short_id);
+        size_t const h2 = std::hash<std::string>{}(value.long_id);
+        return h1 ^ (h2 << 1);
+    }
+};
+
+} // namespace std
+
+namespace sharg::detail
+{
+
+template <typename id_type>
+inline bool id_pair::empty(id_type const & id) noexcept
+{
+    if constexpr (std::same_as<id_type, id_pair>)
+        return id.empty();
+    else if constexpr (std::same_as<id_type, char>)
+        return id == '\0';
+    else
+        return id.empty();
+}
+
+template <typename id_type>
+inline auto id_pair::find(std::unordered_set<id_pair> const & used_ids, id_type const & id)
+{
+    if (empty(id))
+        return used_ids.end();
+
+    return std::ranges::find_if(used_ids,
+                                [&id](id_pair const & pair)
+                                {
+                                    return pair == id;
+                                });
+}
+
+template <typename id_type>
+inline bool id_pair::contains(std::unordered_set<id_pair> const & used_ids, id_type const & id)
+{
+    return find(used_ids, id) != used_ids.end();
+}
+
+} // namespace sharg::detail

include/sharg/parser.hpp

@@ -493,26 +493,22 @@ class parser
         if (!parse_was_called)
             throw design_error{"You can only ask which options have been set after calling the function `parse()`."};
 
-        // the detail::format_parse::find_option_id call in the end expects either a char or std::string
-        using char_or_string_t = std::conditional_t<std::same_as<id_type, char>, char, std::string>;
-        char_or_string_t short_or_long_id = {id}; // e.g. convert char * to string here if necessary
+        detail::id_pair const id_pair{id};
 
-        if constexpr (!std::same_as<id_type, char>) // long id was given
+        if (id_pair.long_id.size() == 1u)
         {
-            if (short_or_long_id.size() == 1)
-            {
-                throw design_error{"Long option identifiers must be longer than one character! If " + short_or_long_id
-                                   + "' was meant to be a short identifier, please pass it as a char ('') not a string"
-                                     " (\"\")!"};
-            }
+            throw design_error{"Long option identifiers must be longer than one character! If " + id_pair.long_id
+                               + "' was meant to be a short identifier, please pass it as a char ('') not a string"
+                                 " (\"\")!"};
         }
 
-        if (!used_ids.contains(std::string{id}))
+        auto const it = detail::id_pair::find(used_option_ids, id_pair);
+        if (it == used_option_ids.end())
             throw design_error{"You can only ask for option identifiers that you added with add_option() before."};
 
         // we only need to search for an option before the `option_end_identifier` (`--`)
-        auto option_end = std::find(format_arguments.begin(), format_arguments.end(), option_end_identifier);
-        auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), option_end, short_or_long_id);
+        auto option_end = std::ranges::find(format_arguments, option_end_identifier);
+        auto option_it = detail::format_parse::find_option_id(format_arguments.begin(), option_end, *it);
         return option_it != option_end;
     }
 
@@ -760,8 +756,13 @@ class parser
                  detail::format_copyright>
         format{detail::format_short_help{}};
 
-    //!\brief List of option/flag identifiers (excluding -/--) that are already used.
-    std::unordered_set<std::string> used_ids{"h", "hh", "help", "advanced-help", "export-help", "version", "copyright"};
+    //!\brief List of option/flag identifiers that are already used.
+    std::unordered_set<detail::id_pair> used_option_ids{{'h', "help"},
+                                                        {'\0' /*hh*/, "advanced-help"},
+                                                        {'\0', "hh"},
+                                                        {'\0', "export-help"},
+                                                        {'\0', "version"},
+                                                        {'\0', "copyright"}};
 
     //!\brief The command line arguments that will be passed to the format.
     std::vector<std::string> format_arguments{};
@@ -963,19 +964,6 @@ class parser
             format = detail::format_parse(format_arguments);
     }
 
-    /*!\brief Checks whether the long identifier has already been used before.
-    * \param[in] id The long identifier of the command line option/flag.
-    * \returns `true` if an option or flag with the long identifier exists or `false`
-    *          otherwise.
-    */
-    template <typename id_type>
-    bool id_exists(id_type const & id)
-    {
-        if (detail::format_parse::is_empty_id(id))
-            return false;
-        return (!(used_ids.insert(std::string({id}))).second);
-    }
-
     /*!\brief Verifies that the short and the long identifiers are correctly formatted.
      * \param[in] short_id The short identifier of the command line option/flag.
      * \param[in] long_id  The long identifier of the command line option/flag.
@@ -1000,7 +988,7 @@ class parser
         {
             if (short_id == '-' || !is_valid(short_id))
                 throw design_error{"Short identifiers may only contain alphanumeric characters, '_', or '@'."};
-            if (id_exists(short_id))
+            if (detail::id_pair::contains(used_option_ids, short_id))
                 throw design_error{"Short identifier '" + std::string(1, short_id) + "' was already used before."};
         }
 
@@ -1012,9 +1000,11 @@ class parser
                 throw design_error{"Long identifiers may not use '-' as first character."};
             if (!std::ranges::all_of(long_id, is_valid))
                 throw design_error{"Long identifiers may only contain alphanumeric characters, '_', '-', or '@'."};
-            if (id_exists(long_id))
+            if (detail::id_pair::contains(used_option_ids, long_id))
                 throw design_error{"Long identifier '" + long_id + "' was already used before."};
         }
+
+        used_option_ids.emplace(short_id, long_id);
     }
 
     //!brief Verify the configuration given to a sharg::parser::add_option call.