Add fmt support for conversion to string

[alepez]

fmt is a high quality, widely used formatting library. It's from the same author of C++20 std::format.

I've used it to add support for ranges to my fork of ApprovalTests.cpp. I've not open a PR, because my current work just fix a single problem (conversion of containers to string) and does not have an adequate quality.

fmt::format can convert to string common types like vector, map, tuple.

To add support for vector or other stl containers, somobody can just add to a project using ApprovalTests.cpp this snippet (or even a more generic one, for all containers)

template <typename T>
std::ostream& operator<<(std::ostream& os, const std::vector<T>& vec) {
  fmt::print(os, "{}", vec);
  return os;
}

But this can cause problems, because we are overloading a foreigner operator for a foreigner library (std in this case), and who knows if another library we are including does the same?.

So my proposal is to add some good support for fmt (and, why not... std::format for C++20 users)

I've already tried a naïve fix, it works for my case, but it breaks some tests in ApprovalTests.cpp (tests don't even compile):

// ApprovalTests/utilities/StringUtils.h
        template <typename T>
        static std::string toString(const T& contents)
        {
            return fmt::format("{}", contents);
        }

At the moment, the best I've done is here, works with standard containers and does not break any test.

[claremacrae]

Thank you!

For info, this relates to (the now rather poorly named) #6...

[alepez]

I've just read the Contributing and the pair programming policy. That's great! I'm ok on doing it, but I have a feeling this is not a simple task which can be solved in one hour session. Tomorrow I'm making some more experiments, I'll let you know.

[claremacrae]

Thanks @alepez ... Don't worry about the 60/90 minute comment... it's not meant to be taken literally...

I think you and I are only 1-hour apart, time-zone-wise, so when you're ready to talk, and it's convenient for you, feel free to drop me a note at the email address in my profile here...

I suspect it will mostly be me learning from you about your changes! 😀

[alepez]

That's great! I'm not so familiar with fmt internal architecture and SFINAE in general (it took me almost an hour to write template <typename T, std::enable_if_t<fmt::is_range<T, char>::value, int>* = nullptr>) and I'm not even sure it's the best way to express it. We may ask @vitaut for some hints, especially if we want a better integration between ApprovalTests and fmt.

[alepez]

Without even touching original ApprovalTests.cpp we can inject the right converter in verifyAll or passing the already converted value to verify.

#include <ApprovalTests.hpp>
#include <gtest/gtest.h>

#include <fmt/format.h>
#include <fmt/ostream.h>
#include <fmt/ranges.h>

using ApprovalTests::Approvals;
using ApprovalTests::StringUtils;

TEST(Verify, FormatVector)
{
    std::vector<int> x{{1, 2, 3}};
    Approvals::verify(fmt::to_string(x));
}

TEST(VerifyAll, FormatVector)
{
    std::vector<std::vector<int>> xs{{1, 2}, {3, 4}, {5, 6}};
    Approvals::verifyAll("", xs, [](const auto& contents, std::ostream& os) {
        fmt::print(os, "{}", contents);
    });
}

[alepez]

After some tests, I've something that can be considered an (optional) integration with fmt.

diff --git a/ApprovalTests/Approvals.h b/ApprovalTests/Approvals.h
index ce3fc06..bc4e811 100644
--- a/ApprovalTests/Approvals.h
+++ b/ApprovalTests/Approvals.h
@@ -153,7 +153,7 @@ namespace ApprovalTests
             verifyAll<std::vector<T>>(
                 header,
                 list,
-                [&](T e, std::ostream& s) { s << "[" << i++ << "] = " << e; },
+                [&](T e, std::ostream& s) { s << "[" << i++ << "] = " << StringUtils::toString(e); },
                 reporter);
         }
 
diff --git a/ApprovalTests/utilities/StringUtils.h b/ApprovalTests/utilities/StringUtils.h
index b2f4f81..1224abd 100644
--- a/ApprovalTests/utilities/StringUtils.h
+++ b/ApprovalTests/utilities/StringUtils.h
@@ -7,6 +7,16 @@
 #include <algorithm>
 #include <sstream>
 
+// TODO Should be enabled by user
+#define TO_STRING_WITH_FMT
+
+#ifdef TO_STRING_WITH_FMT
+#define FMT_HEADER_ONLY
+#include <fmt/format.h>
+#include <fmt/ranges.h>
+#include <fmt/ostream.h>
+#endif
+
 namespace ApprovalTests
 {
     class StringUtils
@@ -52,9 +62,13 @@ namespace ApprovalTests
 
         template <typename T> static std::string toString(const T& contents)
         {
+#ifdef TO_STRING_WITH_FMT
+            return fmt::to_string(contents);
+#else
             std::stringstream s;
             s << contents;
             return s.str();
+#endif
         }
     };
 }
diff --git a/tests/DocTest_Tests/utilities/StringUtilsTests.cpp b/tests/DocTest_Tests/utilities/StringUtilsTests.cpp
index 6c02aa4..8275b70 100644
--- a/tests/DocTest_Tests/utilities/StringUtilsTests.cpp
+++ b/tests/DocTest_Tests/utilities/StringUtilsTests.cpp
@@ -1,9 +1,72 @@
 #include "doctest/doctest.h"
 #include "ApprovalTests/utilities/StringUtils.h"
 
+#ifdef TO_STRING_WITH_FMT
+#include <map>
+#include <tuple>
+#endif
+
 using namespace ApprovalTests;
 
 TEST_CASE("TestLowerCase")
 {
     REQUIRE(StringUtils::toLower("MiXeD CaSe") == "mixed case");
 }
+
+#ifdef TO_STRING_WITH_FMT
+
+TEST_CASE("TestVectorToString")
+{
+    REQUIRE(StringUtils::toString(std::vector<int>{{1, 2, 3}}) == "{1, 2, 3}");
+}
+
+TEST_CASE("TestMapToString")
+{
+    REQUIRE(StringUtils::toString(std::map<std::string, int>{{{"a", 1}, {"b", 2}}}) ==
+            "{(\"a\", 1), (\"b\", 2)}");
+}
+
+TEST_CASE("TestTupleToString")
+{
+    REQUIRE(StringUtils::toString(std::make_tuple(1, 3.0)) == "(1, 3.0)");
+}
+
+class Point
+{
+public:
+    Point(double ax, double ay) : x{ax}, y{ay} {};
+    const double x;
+    const double y;
+};
+
+/*
+ * Instead of implementing operator<< for std::ostream, we can specialize fmt::formatter
+ * for this type.
+ *
+ * This formatter inherits format parameters from the "double"
+ * formatter, so when using fmt::format it uses them to format its members.
+ *
+ *
+ * https://fmt.dev/latest/api.html#formatting-user-defined-types
+ */
+template <> struct fmt::formatter<Point> : fmt::formatter<double>
+{
+    template <typename FormatContext> auto format(const Point& p, FormatContext& ctx)
+    {
+        format_to(ctx.out(), "Point{{ .x = ");
+        formatter<double>::format(p.x, ctx);
+        format_to(ctx.out(), ", .y = ");
+        formatter<double>::format(p.y, ctx);
+        return format_to(ctx.out(), " }}");
+    }
+};
+
+TEST_CASE("TestUserDefinedTypeWithFormatter")
+{
+    Point point{123.456, 3.141592653589};
+    REQUIRE(StringUtils::toString(point) == "Point{ .x = 123.456, .y = 3.141592653589 }");
+    REQUIRE(StringUtils::toString(point) == fmt::format("{}", point));
+    REQUIRE(fmt::format("{:.1f}", point) == "Point{ .x = 123.5, .y = 3.1 }");
+}
+
+#endif

I don't like using macro, but in this case we can just define TO_STRING_WITH_FMT and all default conversion to string provided by fmt works (so standard containers, tuples, etc...).

When this feature is enabled, conversion to string for user defined types can still be defined implementing operator<< on std::ostream, but can also be defined by specialization of fmt::formatter<T>.

[claremacrae]

Thanks @alepez - I'll grab a cup of tea and then clone your fork and try this out!

[claremacrae]

Hi @alepez Thanks - I've experimented with the code, and it makes a lot of sense...

Would love to pair with you - Monday perhaps? - to talk about options for adding it to ApprovalTests.cpp...

[alepez]

Hi @claremacrae, sure! Monday is ok for me, we can continue in private (email) for further organization.

[isidore]

Fixed in v.8.8.0 release