Skip to content

Commit

Permalink
Refactor to combine code for reading width and precision
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
Chris Foster committed Apr 18, 2019
1 parent b2b6ce3 commit 7fde1c3
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 66 deletions.
119 changes: 56 additions & 63 deletions tinyformat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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' ||
Expand Down
8 changes: 5 additions & 3 deletions tinyformat_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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) )

Expand Down

0 comments on commit 7fde1c3

Please sign in to comment.