prioritize wide strings over strings on MSVC (#876)

to handle `std::filesystem::path` better widestring operations should be preferred over regular strings. Fix Issue #875 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
CLIUtils · Jun 15, 2023 · 1939301 · 1939301
1 parent f47ab71
commit 1939301
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 10 deletions.
diff --git a/include/CLI/TypeTools.hpp b/include/CLI/TypeTools.hpp
@@ -607,12 +607,23 @@ template <typename T> struct classify_object<T, typename std::enable_if<is_bool<
 template <typename T> struct classify_object<T, typename std::enable_if<std::is_floating_point<T>::value>::type> {
     static constexpr object_category value{object_category::floating_point};
 };
+#if defined _MSC_VER
+// in MSVC wstring should take precedence if available this isn't as useful on other compilers due to the broader use of
+// utf-8 encoding
+#define WIDE_STRING_CHECK                                                                                              \
+    !std::is_assignable<T &, std::wstring>::value && !std::is_constructible<T, std::wstring>::value
+#define STRING_CHECK true
+#else
+#define WIDE_STRING_CHECK true
+#define STRING_CHECK !std::is_assignable<T &, std::string>::value && !std::is_constructible<T, std::string>::value
+#endif
 
 /// String and similar direct assignment
 template <typename T>
-struct classify_object<T,
-                       typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                                               std::is_assignable<T &, std::string>::value>::type> {
+struct classify_object<
+    T,
+    typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value && WIDE_STRING_CHECK &&
+                            std::is_assignable<T &, std::string>::value>::type> {
     static constexpr object_category value{object_category::string_assignable};
 };
 
@@ -622,28 +633,24 @@ struct classify_object<
     T,
     typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
                             !std::is_assignable<T &, std::string>::value && (type_count<T>::value == 1) &&
-                            std::is_constructible<T, std::string>::value>::type> {
+                            WIDE_STRING_CHECK && std::is_constructible<T, std::string>::value>::type> {
     static constexpr object_category value{object_category::string_constructible};
 };
 
 /// Wide strings
 template <typename T>
 struct classify_object<T,
                        typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                                               !std::is_assignable<T &, std::string>::value &&
-                                               !std::is_constructible<T, std::string>::value &&
-                                               std::is_assignable<T &, std::wstring>::value>::type> {
+                                               STRING_CHECK && std::is_assignable<T &, std::wstring>::value>::type> {
     static constexpr object_category value{object_category::wstring_assignable};
 };
 
 template <typename T>
 struct classify_object<
     T,
     typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                            !std::is_assignable<T &, std::string>::value &&
-                            !std::is_constructible<T, std::string>::value &&
                             !std::is_assignable<T &, std::wstring>::value && (type_count<T>::value == 1) &&
-                            std::is_constructible<T, std::wstring>::value>::type> {
+                            STRING_CHECK && std::is_constructible<T, std::wstring>::value>::type> {
     static constexpr object_category value{object_category::wstring_constructible};
 };
 

diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp
@@ -1715,6 +1715,30 @@ TEST_CASE_METHOD(TApp, "FileExists", "[app]") {
     CHECK(!CLI::ExistingFile(myfile).empty());
 }
 
+#if defined CLI11_HAS_FILESYSTEM && CLI11_HAS_FILESYSTEM > 0 && defined(_MSC_VER)
+TEST_CASE_METHOD(TApp, "filesystemWideName", "[app]") {
+    std::filesystem::path myfile{L"voil\u20ac.txt"};
+
+    std::filesystem::path fpath;
+    app.add_option("--file", fpath)->check(CLI::ExistingFile, "existing file");
+
+    CHECK_THROWS_AS(app.parse(L"--file voil\u20ac.txt"), CLI::ValidationError);
+
+    bool ok = static_cast<bool>(std::ofstream(myfile).put('a'));  // create file
+    CHECK(ok);
+
+    // deactivate the check, so it should run now
+
+    CHECK_NOTHROW(app.parse(L"--file voil\u20ac.txt"));
+
+    CHECK(fpath == myfile);
+
+    CHECK(std::filesystem::exists(fpath));
+    std::filesystem::remove(myfile);
+    CHECK(!std::filesystem::exists(fpath));
+}
+#endif
+
 TEST_CASE_METHOD(TApp, "NotFileExists", "[app]") {
     std::string myfile{"TestNonFileNotUsed.txt"};
     CHECK(!CLI::ExistingFile(myfile).empty());

diff --git a/tests/HelpersTest.cpp b/tests/HelpersTest.cpp
@@ -1033,6 +1033,19 @@ TEST_CASE("Types: TypeName", "[helpers]") {
     CHECK((atomic_name == "INT" || atomic_name == "TEXT"));
 }
 
+TEST_CASE("Types: TypeNameStrings", "[helpers]") {
+    auto sclass = CLI::detail::classify_object<std::string>::value;
+    CHECK(CLI::detail::object_category::string_assignable == sclass);
+
+    auto wsclass = CLI::detail::classify_object<std::wstring>::value;
+    CHECK(CLI::detail::object_category::wstring_assignable == wsclass);
+
+#if defined CLI11_HAS_FILEYSTEM && CLI11_HAS_FILESYSTEM > 0 && defined(_MSC_VER)
+    auto fspclass = CLI::detail::classify_object<std::filesystem::path>::value;
+    CHECK(CLI::detail::object_category::wstring_assignable == fspclass);
+#endif
+}
+
 TEST_CASE("Types: OverflowSmall", "[helpers]") {
     signed char x = 0;
     auto strmax = std::to_string((std::numeric_limits<signed char>::max)() + 1);