Improve format workaround and add unit tests

apps/openmw_test_suite/misc/test_stringops.cpp

@@ -182,4 +182,26 @@ namespace
     {
         EXPECT_FALSE(ciStartsWith("foo", "foo bar"));
     }
+
+    TEST(MiscStringsFormat, string_format)
+    {
+        std::string f = "1%s2";
+        EXPECT_EQ(Misc::StringUtils::format(f, ""), "12");
+    }
+
+    TEST(MiscStringsFormat, string_format_arg)
+    {
+        std::string arg = "12";
+        EXPECT_EQ(Misc::StringUtils::format("1%s2", arg), "1122");
+    }
+
+    TEST(MiscStringsFormat, string_view_format_arg)
+    {
+        std::string f = "1%s2";
+        std::string_view view = "12";
+        EXPECT_EQ(Misc::StringUtils::format(f, view), "1122");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(0, 1)), "112");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(1, 1)), "122");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(2)), "12");
+    }
 }

components/misc/strings/format.hpp

@@ -11,55 +11,65 @@
 
 namespace Misc::StringUtils
 {
-    struct Details
+    namespace Details
     {
-        std::vector<std::string> mMemorySafety;
-
         // Allow to convert complex arguments to C-style strings for format() function
         template <typename T>
         T argument(T value) noexcept
         {
+            static_assert(!std::is_same_v<T, std::string_view>, "std::string_view is not supported");
             return value;
         }
 
-        template <typename T>
-        T const * argument(std::basic_string_view<T> const & value) noexcept
-        {
-            // TODO: switch to a format function that doesn't require null termination
-            auto& inserted = mMemorySafety.emplace_back(value);
-            return inserted.c_str();
-        }
-
         template <typename T>
         T const * argument(std::basic_string<T> const & value) noexcept
         {
             return value.c_str();
         }
-    };
 
-    // Requires some C++11 features:
+        template<class T>
-    // 1. std::string needs to be contiguous
+        T nullTerminated(T value) noexcept
-    // 2. std::snprintf with zero size (second argument) returns an output string size
+        {
-    // 3. variadic templates support
+            return value;
+        }
+
+        template<class T>
+        std::basic_string<T> nullTerminated(const std::basic_string_view<T>& value) noexcept
+        {
+            // Ensure string_view arguments are null-terminated by creating a string
+            // TODO: Use a format function that doesn't require this workaround
+            return std::string{value};
+        }
+
+        // Requires some C++11 features:
+        // 1. std::string needs to be contiguous
+        // 2. std::snprintf with zero size (second argument) returns an output string size
+        // 3. variadic templates support
+        template <typename ... Args>
+        std::string format(const char* fmt, Args const & ... args)
+        {
+            const int size = std::snprintf(nullptr, 0, fmt, argument(args) ...);
+            if (size < 0)
+                throw std::runtime_error(std::string("Failed to compute resulting string size: ") + std::strerror(errno));
+            // Note: sprintf also writes a trailing null character. We should remove it.
+            std::string ret(static_cast<std::size_t>(size) + 1, '\0');
+            if (std::sprintf(ret.data(), fmt, argument(args) ...) < 0)
+                throw std::runtime_error(std::string("Failed to format string: ") + std::strerror(errno));
+            ret.erase(static_cast<std::size_t>(size));
+            return ret;
+        }
+    }
+
     template <typename ... Args>
     std::string format(const char* fmt, Args const & ... args)
     {
-        Details details;
+        return Details::format(fmt, Details::nullTerminated(args) ...);
-        const int size = std::snprintf(nullptr, 0, fmt, details.argument(args) ...);
-        if (size < 0)
-            throw std::runtime_error(std::string("Failed to compute resulting string size: ") + std::strerror(errno));
-        // Note: sprintf also writes a trailing null character. We should remove it.
-        std::string ret(static_cast<std::size_t>(size) + 1, '\0');
-        if (std::sprintf(ret.data(), fmt, details.argument(args) ...) < 0)
-            throw std::runtime_error(std::string("Failed to format string: ") + std::strerror(errno));
-        ret.erase(static_cast<std::size_t>(size));
-        return ret;
     }
 
     template <typename ... Args>
     std::string format(const std::string& fmt, Args const & ... args)
     {
-        return format(fmt.c_str(), args ...);
+        return Details::format(fmt.c_str(), Details::nullTerminated(args) ...);
     }
 }