Refactor to combine code for reading width and precision

The positional mode code makes this complex enough that it seems worth
combining these in one place.

Also add an extra test to check that negative variable position is
treated as though no precision was set.

Author: Chris Foster

tinyformat.h

@@ -555,6 +555,49 @@ inline int parseIntAndAdvance(const char*& c)
     return i;
 }
 
+// Parse width or precision `n` from format string pointer `c`, and advance it
+// to the next character. If an indirection is requested with `*`, the argument
+// is read from `formatters[argIndex]` and `argIndex` is incremented (or read
+// from `formatters[n]` in positional mode). Returns true if one or more
+// characters were read.
+inline bool parseWidthOrPrecision(int& n, const char*& c, bool positionalMode,
+                                  const detail::FormatArg* formatters,
+                                  int& argIndex, int numFormatters)
+{
+    if(*c >= '0' && *c <= '9')
+    {
+        n = parseIntAndAdvance(c);
+    }
+    else if(*c == '*')
+    {
+        ++c;
+        n = 0;
+        if(positionalMode)
+        {
+            int pos = parseIntAndAdvance(c) - 1;
+            if(*c != '$')
+                TINYFORMAT_ERROR("tinyformat: Non-positional argument used after a positional one");
+            if(pos >= 0 && pos < numFormatters)
+                n = formatters[pos].toInt();
+            else
+                TINYFORMAT_ERROR("tinyformat: Positional argument out of range");
+            ++c;
+        }
+        else
+        {
+            if(argIndex < numFormatters)
+                n = formatters[argIndex++].toInt();
+            else
+                TINYFORMAT_ERROR("tinyformat: Not enough arguments to read variable width or precision");
+        }
+    }
+    else
+    {
+        return false;
+    }
+    return true;
+}
+
 // Print literal part of format string and return next format spec
 // position.
 //
@@ -649,15 +692,12 @@ inline const char* streamStateFromFormat(std::ostream& out, bool& positionalMode
         const char tmpc = *c;
         int value = parseIntAndAdvance(c);
         if(*c == '$')
-        {  // value is an argument index
+        {
+            // value is an argument index
             if(value > 0 && value <= numFormatters)
-            {
                 argIndex = value - 1;
-            }
             else
-            {
                 TINYFORMAT_ERROR("tinyformat: Positional argument out of range");
-            }
             ++c;
             positionalMode = true;
         }
@@ -727,33 +767,11 @@ inline const char* streamStateFromFormat(std::ostream& out, bool& positionalMode
             break;
         }
         // Parse width
-        if(*c >= '0' && *c <= '9')
-        {
-            widthSet = true;
-            out.width(parseIntAndAdvance(c));
-        }
-        else if(*c == '*')
+        int width = 0;
+        widthSet = parseWidthOrPrecision(width, c, positionalMode,
+                                         formatters, argIndex, numFormatters);
+        if(widthSet)
         {
-            widthSet = true;
-            int width = 0;
-            if(positionalMode)
-            {
-                ++c;
-                int pos = parseIntAndAdvance(c) - 1;
-                if(*c != '$')
-                    TINYFORMAT_ERROR("tinyformat: Non-positional argument used after a positional one");
-                if(pos >= 0 && pos < numFormatters)
-                    width = formatters[pos].toInt();
-                else
-                    TINYFORMAT_ERROR("tinyformat: Positional argument out of range");
-            }
-            else
-            {
-                if(argIndex < numFormatters)
-                    width = formatters[argIndex++].toInt();
-                else
-                    TINYFORMAT_ERROR("tinyformat: Not enough arguments to read variable width");
-            }
             if(width < 0)
             {
                 // negative widths correspond to '-' flag set
@@ -762,45 +780,20 @@ inline const char* streamStateFromFormat(std::ostream& out, bool& positionalMode
                 width = -width;
             }
             out.width(width);
-            ++c;
         }
     }
     // 3) Parse precision
     if(*c == '.')
     {
         ++c;
         int precision = 0;
-        if(*c == '*')
-        {
-            ++c;
-            if(positionalMode)
-            {
-                int pos = parseIntAndAdvance(c) - 1;
-                if(*c != '$')
-                    TINYFORMAT_ERROR("tinyformat: Non-positional argument used after a positional one");
-                if(pos >= 0 && pos < numFormatters)
-                    precision = formatters[pos].toInt();
-                else
-                    TINYFORMAT_ERROR("tinyformat: Positional argument out of range");
-                ++c;
-            }
-            else
-            {
-                if(argIndex < numFormatters)
-                    precision = formatters[argIndex++].toInt();
-                else
-                    TINYFORMAT_ERROR("tinyformat: Not enough arguments to read variable precision");
-            }
-        }
-        else
-        {
-            if(*c >= '0' && *c <= '9')
-                precision = parseIntAndAdvance(c);
-            else if(*c == '-') // negative precisions ignored, treated as zero.
-                parseIntAndAdvance(++c);
-        }
-        out.precision(precision);
-        precisionSet = true;
+        parseWidthOrPrecision(precision, c, positionalMode,
+                              formatters, argIndex, numFormatters);
+        // Presence of `.` indicates precision set, unless the inferred value
+        // was negative in which case the default is used.
+        precisionSet = precision >= 0;
+        if(precisionSet)
+            out.precision(precision);
     }
     // 4) Ignore any C99 length modifier
     while(*c == 'l' || *c == 'h' || *c == 'L' ||

tinyformat_test.cpp

@@ -128,9 +128,9 @@ int unitTests()
     CHECK_EQUAL(tfm::format("%hc", (short)65), "A");
     CHECK_EQUAL(tfm::format("%lc", (long)65), "A");
     CHECK_EQUAL(tfm::format("%s", "asdf_123098"), "asdf_123098");
-    // Note: All tests printing pointers are different on windows, since
-    // there's no standard numerical representation.
-    // Representation also differs between 32-bit and 64-bit windows.
+
+    // Test printing of pointers.  Note that there's no standard numerical
+    // representation so this is platform and OS dependent.
 #   ifdef _MSC_VER
 #   ifdef _WIN64
     CHECK_EQUAL(tfm::format("%p", (void*)0x12345), "0000000000012345");
@@ -170,6 +170,7 @@ int unitTests()
     CHECK_EQUAL(tfm::format("%10.*f", 4, 1234.1234567890), " 1234.1235");
     CHECK_EQUAL(tfm::format("%*.*f", 10, 4, 1234.1234567890), " 1234.1235");
     CHECK_EQUAL(tfm::format("%*.*f", -10, 4, 1234.1234567890), "1234.1235 ");
+    CHECK_EQUAL(tfm::format("%.*f", -4, 1234.1234567890), "1234.123457"); // negative precision ignored
     // Test variable precision & width with positional arguments
     CHECK_EQUAL(tfm::format("%1$*2$.4f", 1234.1234567890, 10), " 1234.1235");
     CHECK_EQUAL(tfm::format("%1$10.*2$f", 1234.1234567890, 4), " 1234.1235");
@@ -238,6 +239,7 @@ int unitTests()
     EXPECT_ERROR( tfm::format("%0$d", 1)      )
     EXPECT_ERROR( tfm::format("%1$.*3$d", 1, 2)     )
     EXPECT_ERROR( tfm::format("%1$.*0$d", 1, 2)     )
+    EXPECT_ERROR( tfm::format("%1$.*$d",  1, 2)     )
     EXPECT_ERROR( tfm::format("%3$*4$.*2$d", 1, 2, 3) )
     EXPECT_ERROR( tfm::format("%3$*0$.*2$d", 1, 2, 3) )