Fixed bugs in MiniFormat

- The 'p' specifier didn't work at all (compile error)
- Improved floating-point formatting when FLOAT_TO_CHARS_AVAILABLE=0,
  enough to pass the tests

include/crouton/util/MiniFormat.hh

@@ -26,7 +26,7 @@
 
 /*
  A string formatting API mostly compatible with `std::format`, but optimized for small code size.
- https://en.cppreference.com/w/cpp/utility/format/formatter
+ https://en.cppreference.com/w/cpp/utility/format/spec
 
  Missing functionality and limitations:
  - You can't create custom formatters that interpret custom field specs. (But you can format
@@ -85,7 +85,7 @@ namespace crouton::mini {
             "cbBdoxX",      // Char
             "bBcdoxX", "bBcdoxX", "bBcdoxX", "bBcdoxX", "bBcdoxX", "bBcdoxX",  // integers
             "aAeEfFgG",     // Double
-            "s", "s", "s"   // strings
+            "s", "s", "s",  // strings
             "pP",           // Pointer
             "s"             // Arg
         };

src/support/MiniFormat.cc

@@ -132,14 +132,15 @@ namespace crouton::mini {
     {
         if (d >= 0.0)
             writeNonNegativeSign(out, spec.sign);
+        bool precise = (spec.precision != BaseFormatString::kDefaultPrecision);
+        int precision = precise ? spec.precision : 6;
         char buf[60];   // big enough for all but absurdly large precisions
 #if FLOAT_TO_CHARS_AVAILABLE
         std::to_chars_result result;
-        if (spec.type == 0 && spec.precision == BaseFormatString::kDefaultPrecision) {
+        if (spec.type == 0 && !precise) {
             result = std::to_chars(&buf[0], &buf[sizeof(buf)], d);
         } else {
             std::chars_format format;
-            int precision = (spec.precision != BaseFormatString::kDefaultPrecision) ? spec.precision : 6;
             switch (spec.type) {
                 case 'a': case 'A': format = std::chars_format::hex; break;
                 case 'e': case 'E': format = std::chars_format::scientific; break;
@@ -164,9 +165,53 @@ namespace crouton::mini {
         }
 
         out.write(&buf[0], result.ptr);
+
 #else
-        // lame fallback if to_chars isn't available:
-        snprintf(buf, sizeof(buf), "%g", d);
+        // fallback if floating-point std::to_chars isn't available:
+        switch (spec.type) {
+            case 'a':
+                snprintf(buf, sizeof(buf), "%.*a", precision, d);
+                if (auto x = strstr(buf, "0x"))
+                    ::memmove(x, x + 2, strlen(x));
+                break;
+            case 'A':
+                snprintf(buf, sizeof(buf), "%.*A", precision, d);
+                if (auto x = strstr(buf, "0X"))
+                    ::memmove(x, x + 2, strlen(x));
+                break;
+            case 'e':   snprintf(buf, sizeof(buf), "%.*e", precision, d); break;
+            case 'E':   snprintf(buf, sizeof(buf), "%.*E", precision, d); break;
+            case 'f':
+            case 'F':   snprintf(buf, sizeof(buf), "%.*f", precision, d); break;
+            case 'g':   snprintf(buf, sizeof(buf), "%.*g", precision, d); break;
+            case 'G':   snprintf(buf, sizeof(buf), "%.*G", precision, d); break;
+            case 0:
+                if (precise) {
+                    snprintf(buf, sizeof(buf), "%.*g", precision, d);
+                } else {
+                    if (auto x = log10(abs(d)); d == 0.0 || (x > -5 && x < 10))
+                        snprintf(buf, sizeof(buf), "%.8f", d);
+                    else
+                        snprintf(buf, sizeof(buf), "%.8e", d);
+                }
+                break;
+            default:
+                throw format_error("invalid format type for vformat_double");
+        }
+        if ((spec.type == 'g' || spec.type == 0) && !precise) {
+            // Trim trailing 0s after decimal point, and a trailing decimal point:
+            if (strchr(buf, '.') && !strchr(buf, 'e')) {
+                for (auto p = strlen(buf) - 1; buf[p] == '0'; --p)
+                    buf[p] = '\0';
+            }
+            if (auto& last = buf[strlen(buf) - 1]; last == '.')
+                last = '\0';
+        }
+        if (spec.alternate) {
+            // Alt '#' spec means to ensure there's a decimal point:
+            if (!strchr(buf, '.'))
+                strcat(buf, ".");
+        }
         out.write(buf);
 #endif
     }

tests/test_mini.cc

@@ -24,21 +24,27 @@ TEST_CASE("FormatString Spec", "[mini]") {
     InitLogging(); //FIXME: Put this somewhere where it gets run before any test
     using enum mini::BaseFormatString::align_t;
     using enum mini::BaseFormatString::sign_t;
+    using enum mini::i::ArgType;
 
-    static constexpr struct {const char* str; mini::BaseFormatString::Spec spec;} kTests[] = {
-        {"{}", {}},
-        {"{:}", {}},
-        {"{:d}", {.type = 'd'}},
-        {"{:^}", {.fill = ' ', .align = center}},
-        {"{:*>}", {.fill = '*', .align = right}},
-        {"{:+0}", {.fill = '0', .align = right, .sign = minusPlus}},
-        {"{:+3.4f}", {.type = 'f', .width = 3, .precision = 4, .sign = minusPlus}},
-        {"{:+0.4z}", {.type = 'z', .fill = '0', .precision = 4, .align = right, .sign = minusPlus}},
+    static constexpr struct {
+        const char* str;
+        mini::i::ArgType argType;
+        mini::BaseFormatString::Spec spec;
+    } kTests[] = {
+        {"{}",      None, {}},
+        {"{:}",     None, {}},
+        {"{:d}",    Int, {.type = 'd', .align = right}},
+        {"{:p}",    Pointer, {.type = 'p'}},
+        {"{:^}",    None, {.fill = ' ', .align = center}},
+        {"{:*>}",   None, {.fill = '*', .align = right}},
+        {"{:+0}",   None, {.fill = '0', .align = right, .sign = minusPlus}},
+        {"{:+3.4f}",Double, {.type = 'f', .width = 3, .precision = 4, .align = right, .sign = minusPlus}},
+        {"{:+0.4a}", Double, {.type = 'a', .fill = '0', .precision = 4, .align = right, .sign = minusPlus}},
     };
     for (auto const& test : kTests) {
         INFO("Testing " << test.str);
         mini::BaseFormatString::Spec spec;
-        spec.parse(test.str + 1, mini::i::ArgType::None);
+        spec.parse(test.str + 1, test.argType);
         CHECK(spec == test.spec);
     }
 }
@@ -136,6 +142,8 @@ TEST_CASE("MiniFormat", "[mini]") {
     cstr = nullptr;
     CHECK(mini::format("cstr '{}'", cstr)  == "cstr ''");
 
+    CHECK(mini::format("ptr {:p}", (const void*)0x1234)  == "ptr 0x1234");
+
     // excess args:
     CHECK(mini::format("One {} two {} three", 1, 2, 3)
           == "One 1 two 2 three : 3");
@@ -241,6 +249,7 @@ TEST_CASE("MiniFormat Floats", "[mini]") {
         {"{:-}",  -1234.5678,  "-1234.5678"},
         {"{:+}",  -1234.5678,  "-1234.5678"},
         {"{: }",  -1234.5678,  "-1234.5678"},
+        {"{:}",    0.0,        "0"},
         {"{:-}",   0.0,        "0"},
         {"{:+}",   0.0,        "+0"},
         {"{: }",   0.0,        " 0"},