Skip to content

Commit

Permalink
Merge pull request #7112 from shove70/patch-format
Browse files Browse the repository at this point in the history 
Fix issue 20064,20069 - format functions make their final width and digits grouping conform to POSIX definition
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
  • Loading branch information
@dlang-bot
dlang-bot committed Aug 4, 2019
2 parents cc9a56f + 75f65a3 commit f54c9fc
Showing 1 changed file with 151 additions and 66 deletions.
217 changes: 151 additions & 66 deletions std/format.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ uint formattedWrite(Writer, Char, A...)(auto ref Writer w, const scope Char[] fm
@safe unittest
{
assert(format("%,d", 1000) == "1,000");
assert(format("%,f", 1234567.891011) == "1,234,567.891,011");
assert(format("%,f", 1234567.891011) == "1,234,567.891011");
assert(format("%,?d", '?', 1000) == "1?000");
assert(format("%,1d", 1000) == "1,0,0,0", format("%,1d", 1000));
assert(format("%,*d", 4, -12345) == "-1,2345");
Expand Down Expand Up @@ -2359,14 +2359,29 @@ private void formatUnsigned(Writer, T, Char)
size_t leftpad = 0;
size_t rightpad = 0;

immutable size_t dlen = digits.length == 0 ? 0 : digits.length - 1;
immutable prefixWidth = (prefix1 != 0) + (prefix2 != 0);
size_t finalWidth, separatorsCount;
if (fs.flSeparator != 0)
{
finalWidth = prefixWidth + digits.length + ((digits.length > 0) ? (digits.length - 1) / fs.separators : 0);
if (finalWidth < fs.width)
finalWidth = fs.width + (padChar == '0') * (((fs.width - prefixWidth) % (fs.separators + 1) == 0) ? 1 : 0);

separatorsCount = (padChar == '0') ? (finalWidth - prefixWidth - 1) / (fs.separators + 1) :
((digits.length > 0) ? (digits.length - 1) / fs.separators : 0);
}
else
{
import std.algorithm.comparison : max;
finalWidth = max(fs.width, prefixWidth + digits.length);
}

immutable ptrdiff_t spacesToPrint =
fs.width - (
(prefix1 != 0)
+ (prefix2 != 0)
finalWidth - (
+ prefixWidth
+ zerofill
+ digits.length
+ ((fs.flSeparator != 0) * (dlen / fs.separators))
+ separatorsCount
);
if (spacesToPrint > 0) // need to do some padding
{
Expand All @@ -2393,18 +2408,14 @@ private void formatUnsigned(Writer, T, Char)
--zerofill;
}

int j = fs.width;
int j = cast(int) (finalWidth - prefixWidth - separatorsCount - 1);
for (size_t i = 0; i < zerofill; ++i, --j)
{
if (j != fs.width && j % fs.separators == 0)
if (j % fs.separators == 0)
{
put(w, fs.separatorChar);
++i;
}
if (i < zerofill)
{
put(w, '0');
}
put(w, '0');
}
}
else
Expand All @@ -2417,7 +2428,7 @@ private void formatUnsigned(Writer, T, Char)
{
for (size_t j = 0; j < digits.length; ++j)
{
if (j != 0 && (digits.length - j) % fs.separators == 0)
if (((j != 0) || ((spacesToPrint > 0) && (padChar == '0'))) && (digits.length - j) % fs.separators == 0)
{
put(w, fs.separatorChar);
}
Expand Down Expand Up @@ -2513,6 +2524,42 @@ private void formatUnsigned(Writer, T, Char)
assert(result == "9");
}

// bugzilla 20064
@safe unittest
{
assert(format( "%03,d", 1234) == "1,234");
assert(format( "%04,d", 1234) == "1,234");
assert(format( "%05,d", 1234) == "1,234");
assert(format( "%06,d", 1234) == "01,234");
assert(format( "%07,d", 1234) == "001,234");
assert(format( "%08,d", 1234) == "0,001,234");
assert(format( "%09,d", 1234) == "0,001,234");
assert(format("%010,d", 1234) == "00,001,234");
assert(format("%011,d", 1234) == "000,001,234");
assert(format("%012,d", 1234) == "0,000,001,234");
assert(format("%013,d", 1234) == "0,000,001,234");
assert(format("%014,d", 1234) == "00,000,001,234");
assert(format("%015,d", 1234) == "000,000,001,234");
assert(format("%016,d", 1234) == "0,000,000,001,234");
assert(format("%017,d", 1234) == "0,000,000,001,234");

assert(format( "%03,d", -1234) == "-1,234");
assert(format( "%04,d", -1234) == "-1,234");
assert(format( "%05,d", -1234) == "-1,234");
assert(format( "%06,d", -1234) == "-1,234");
assert(format( "%07,d", -1234) == "-01,234");
assert(format( "%08,d", -1234) == "-001,234");
assert(format( "%09,d", -1234) == "-0,001,234");
assert(format("%010,d", -1234) == "-0,001,234");
assert(format("%011,d", -1234) == "-00,001,234");
assert(format("%012,d", -1234) == "-000,001,234");
assert(format("%013,d", -1234) == "-0,000,001,234");
assert(format("%014,d", -1234) == "-0,000,001,234");
assert(format("%015,d", -1234) == "-00,000,001,234");
assert(format("%016,d", -1234) == "-000,000,001,234");
assert(format("%017,d", -1234) == "-0,000,000,001,234");
}

private enum ctfpMessage = "Cannot format floating point types at compile-time";

/*
Expand Down Expand Up @@ -2621,45 +2668,57 @@ if (is(FloatingPointTypeOf!T) && !is(T == enum) && !hasToString!(T, Char))
enforceFmt(n >= 0,
"floating point formatting failure");

auto len = min(n, buf.length-1);
ptrdiff_t dot = buf[0 .. len].indexOf('.');
size_t len = min(n, buf.length-1);
if (fs.flSeparator)
{
ptrdiff_t firstDigit = buf[0 .. len].indexOfAny("0123456789");
ptrdiff_t ePos = buf[0 .. len].indexOf('e');
auto dotIdx = dot == -1 ? ePos == -1 ? len : ePos : dot;
size_t j;

ptrdiff_t firstLen = dotIdx - firstDigit;
ptrdiff_t indexOfRemovable()
{
if (len < 2)
return -1;

size_t separatorScoreCnt = (firstLen > 0) ? (firstLen - 1) / fs.separators : 0;
size_t start = (buf[0 .. 1].indexOfAny(" 0123456789") == -1) ? 1 : 0;
if (len < 2 + start)
return -1;
if ((buf[start] == ' ') || (buf[start] == '0' && buf[start + 1] != '.'))
return start;

size_t afterDotIdx;
if (ePos != -1)
{
afterDotIdx = ePos;
}
else
{
afterDotIdx = len;
return -1;
}

if (dot != -1)
ptrdiff_t dot, firstDigit, ePos, dotIdx, firstLen;
size_t separatorScoreCnt;

while (true)
{
ptrdiff_t mantissaLen = afterDotIdx - (dot + 1);
separatorScoreCnt += (mantissaLen > 0) ? (mantissaLen - 1) / fs.separators : 0;
dot = buf[0 .. len].indexOf('.');
firstDigit = buf[0 .. len].indexOfAny("0123456789");
ePos = buf[0 .. len].indexOf('e');
dotIdx = dot == -1 ? ePos == -1 ? len : ePos : dot;

firstLen = dotIdx - firstDigit;
separatorScoreCnt = (firstLen > 0) ? (firstLen - 1) / fs.separators : 0;

ptrdiff_t removableIdx = (len + separatorScoreCnt > fs.width) ? indexOfRemovable() : -1;
if ((removableIdx != -1) &&
((firstLen - (buf[removableIdx] == '0' ? 2 : 1)) / fs.separators + len - 1 >= fs.width))
{
buf[removableIdx .. $ - 1] = buf.dup[removableIdx + 1 .. $];
len--;
}
else
break;
}

// plus, minus prefix
ptrdiff_t digitsBegin = buf[0 .. separatorScoreCnt].indexOfNeither(" ");
if (digitsBegin == -1)
immutable afterDotIdx = (ePos != -1) ? ePos : len;

// plus, minus, prefix
if (firstDigit > 0)
{
digitsBegin = separatorScoreCnt;
put(w, buf[0 .. firstDigit]);
}
put(w, buf[digitsBegin .. firstDigit]);

// digits until dot with separator
for (j = 0; j < firstLen; ++j)
for (auto j = 0; j < firstLen; ++j)
{
if (j > 0 && (firstLen - j) % fs.separators == 0)
{
Expand All @@ -2675,13 +2734,8 @@ if (is(FloatingPointTypeOf!T) && !is(T == enum) && !hasToString!(T, Char))
}

// digits after dot
for (j = dotIdx + 1; j < afterDotIdx; ++j)
for (auto j = dotIdx + 1; j < afterDotIdx; ++j)
{
auto realJ = (j - (dotIdx + 1));
if (realJ != 0 && realJ % fs.separators == 0)
{
put(w, fs.separatorChar);
}
put(w, buf[j]);
}

Expand All @@ -2702,7 +2756,7 @@ if (is(FloatingPointTypeOf!T) && !is(T == enum) && !hasToString!(T, Char))
assert(format("%.1f", 1337.7) == "1337.7");
assert(format("%,3.2f", 1331.982) == "1,331.98");
assert(format("%,3.0f", 1303.1982) == "1,303");
assert(format("%#,3.4f", 1303.1982) == "1,303.198,2");
assert(format("%#,3.4f", 1303.1982) == "1,303.1982");
assert(format("%#,3.0f", 1303.1982) == "1,303.");
}

Expand Down Expand Up @@ -2752,6 +2806,37 @@ if (is(FloatingPointTypeOf!T) && !is(T == enum) && !hasToString!(T, Char))
assert(format("^%13,3.2f$", 10_000_000.00) == "^10,000,000.00$");
}

// bugzilla 20069
@safe unittest
{
assert(format("%012,f", -1234.0) == "-1,234.000000");
assert(format("%013,f", -1234.0) == "-1,234.000000");
assert(format("%014,f", -1234.0) == "-01,234.000000");
assert(format("%011,f", 1234.0) == "1,234.000000");
assert(format("%012,f", 1234.0) == "1,234.000000");
assert(format("%013,f", 1234.0) == "01,234.000000");
assert(format("%014,f", 1234.0) == "001,234.000000");
assert(format("%015,f", 1234.0) == "0,001,234.000000");
assert(format("%016,f", 1234.0) == "0,001,234.000000");

assert(format( "%08,.2f", -1234.0) == "-1,234.00");
assert(format( "%09,.2f", -1234.0) == "-1,234.00");
assert(format("%010,.2f", -1234.0) == "-01,234.00");
assert(format("%011,.2f", -1234.0) == "-001,234.00");
assert(format("%012,.2f", -1234.0) == "-0,001,234.00");
assert(format("%013,.2f", -1234.0) == "-0,001,234.00");
assert(format("%014,.2f", -1234.0) == "-00,001,234.00");
assert(format( "%08,.2f", 1234.0) == "1,234.00");
assert(format( "%09,.2f", 1234.0) == "01,234.00");
assert(format("%010,.2f", 1234.0) == "001,234.00");
assert(format("%011,.2f", 1234.0) == "0,001,234.00");
assert(format("%012,.2f", 1234.0) == "0,001,234.00");
assert(format("%013,.2f", 1234.0) == "00,001,234.00");
assert(format("%014,.2f", 1234.0) == "000,001,234.00");
assert(format("%015,.2f", 1234.0) == "0,000,001,234.00");
assert(format("%016,.2f", 1234.0) == "0,000,001,234.00");
}

/*
Formatting a `creal` is deprecated but still kept around for a while.
*/
Expand Down Expand Up @@ -6625,28 +6710,28 @@ char[] sformat(Char, Args...)(return scope char[] buf, scope const(Char)[] fmt,
@safe unittest
{
auto tmp = format("%,f", 1000.0);
assert(tmp == "1,000.000,000", "'" ~ tmp ~ "'");
assert(tmp == "1,000.000000", "'" ~ tmp ~ "'");

tmp = format("%,f", 1234567.891011);
assert(tmp == "1,234,567.891,011", "'" ~ tmp ~ "'");
assert(tmp == "1,234,567.891011", "'" ~ tmp ~ "'");

tmp = format("%,f", -1234567.891011);
assert(tmp == "-1,234,567.891,011", "'" ~ tmp ~ "'");
assert(tmp == "-1,234,567.891011", "'" ~ tmp ~ "'");

tmp = format("%,2f", 1234567.891011);
assert(tmp == "1,23,45,67.89,10,11", "'" ~ tmp ~ "'");
assert(tmp == "1,23,45,67.891011", "'" ~ tmp ~ "'");

tmp = format("%18,f", 1234567.891011);
assert(tmp == " 1,234,567.891,011", "'" ~ tmp ~ "'");
assert(tmp == " 1,234,567.891011", "'" ~ tmp ~ "'");

tmp = format("%18,?f", '.', 1234567.891011);
assert(tmp == " 1.234.567.891.011", "'" ~ tmp ~ "'");
assert(tmp == " 1.234.567.891011", "'" ~ tmp ~ "'");

tmp = format("%,?.3f", 'ä', 1234567.891011);
assert(tmp == "1ä234ä567.891", "'" ~ tmp ~ "'");

tmp = format("%,*?.3f", 1, 'ä', 1234567.891011);
assert(tmp == "1ä2ä3ä4ä5ä6ä7.8ä9ä1", "'" ~ tmp ~ "'");
assert(tmp == "1ä2ä3ä4ä5ä6ä7.891", "'" ~ tmp ~ "'");

tmp = format("%,4?.3f", '_', 1234567.891011);
assert(tmp == "123_4567.891", "'" ~ tmp ~ "'");
Expand All @@ -6655,19 +6740,19 @@ char[] sformat(Char, Args...)(return scope char[] buf, scope const(Char)[] fmt,
assert(tmp == " 1,234.568", "'" ~ tmp ~ "'");

tmp = format("%,e", 3.141592653589793238462);
assert(tmp == "3.141,593e+00", "'" ~ tmp ~ "'");
assert(tmp == "3.141593e+00", "'" ~ tmp ~ "'");

tmp = format("%15,e", 3.141592653589793238462);
assert(tmp == " 3.141,593e+00", "'" ~ tmp ~ "'");
assert(tmp == " 3.141593e+00", "'" ~ tmp ~ "'");

tmp = format("%15,e", -3.141592653589793238462);
assert(tmp == " -3.141,593e+00", "'" ~ tmp ~ "'");
assert(tmp == " -3.141593e+00", "'" ~ tmp ~ "'");

tmp = format("%.4,*e", 2, 3.141592653589793238462);
assert(tmp == "3.14,16e+00", "'" ~ tmp ~ "'");
assert(tmp == "3.1416e+00", "'" ~ tmp ~ "'");

tmp = format("%13.4,*e", 2, 3.141592653589793238462);
assert(tmp == " 3.14,16e+00", "'" ~ tmp ~ "'");
assert(tmp == " 3.1416e+00", "'" ~ tmp ~ "'");

tmp = format("%,.0f", 3.14);
assert(tmp == "3", "'" ~ tmp ~ "'");
Expand All @@ -6676,7 +6761,7 @@ char[] sformat(Char, Args...)(return scope char[] buf, scope const(Char)[] fmt,
assert(tmp == "1e+06", "'" ~ tmp ~ "'");

tmp = format("%19,?f", '.', -1234567.891011);
assert(tmp == " -1.234.567.891.011", "'" ~ tmp ~ "'");
assert(tmp == " -1.234.567.891011", "'" ~ tmp ~ "'");
}

// Test for multiple indexes
Expand Down Expand Up @@ -6708,23 +6793,23 @@ char[] sformat(Char, Args...)(return scope char[] buf, scope const(Char)[] fmt,
tmp = format("%04d", 100);
assert(tmp == cmp, tmp);

cmp = "0000,000,100";
cmp = "0,000,000,100";
tmp = format("%012,3d", 100);
assert(tmp == cmp, tmp);

cmp = "0000,001,000";
cmp = "0,000,001,000";
tmp = format("%012,3d", 1_000);
assert(tmp == cmp, tmp);

cmp = "0000,100,000";
cmp = "0,000,100,000";
tmp = format("%012,3d", 100_000);
assert(tmp == cmp, tmp);

cmp = "0001,000,000";
cmp = "0,001,000,000";
tmp = format("%012,3d", 1_000_000);
assert(tmp == cmp, tmp);

cmp = "0100,000,000";
cmp = "0,100,000,000";
tmp = format("%012,3d", 100_000_000);
assert(tmp == cmp, tmp);
}
Expand All @@ -6740,7 +6825,7 @@ char[] sformat(Char, Args...)(return scope char[] buf, scope const(Char)[] fmt,
tmp = format("%07,d", 100_000);
assert(tmp == cmp, tmp);

cmp = "0100,000";
cmp = "0,100,000";
tmp = format("%08,d", 100_000);
assert(tmp == cmp, tmp);
}

0 comments on commit f54c9fc

Please sign in to comment.