Skip to content

Commit 28c65bb

Browse files
jasondaviesjasondavies
committed
Fix group formatting and padding.
Very small or large numbers use exponent notation when converted to strings. If grouping is enabled and there is no decimal, the exponent wasn’t being ignored, so the group separator was in the wrong place. This also fixes a bug relating to padding and thousands grouping: the padding calculation assumed a grouping of [3]. Fixes d3#1972, d3#1994.
1 parent f38ed05 commit 28c65bb

4 files changed

Lines changed: 71 additions & 23 deletions

File tree

‎d3.js‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,21 +2120,22 @@
21202120
};
21212121
}
21222122
function d3_locale_numberFormat(locale) {
2123-
var locale_decimal = locale.decimal, locale_thousands = locale.thousands, locale_grouping = locale.grouping, locale_currency = locale.currency, formatGroup = locale_grouping ? function(value) {
2124-
var i = value.length, t = [], j = 0, g = locale_grouping[0];
2125-
while (g > 0 && i > 0) {
2123+
var locale_decimal = locale.decimal, locale_thousands = locale.thousands, locale_grouping = locale.grouping, locale_currency = locale.currency, formatGroup = locale_grouping && locale_thousands ? function(value, width) {
2124+
var i = value.length, t = [], j = 0, g = locale_grouping[0], length = 0;
2125+
while (i > 0 && g > 0) {
2126+
if (length + g + 1 > width) g = Math.max(1, width - length);
21262127
t.push(value.substring(i -= g, i + g));
2128+
if ((length += g + 1) > width) break;
21272129
g = locale_grouping[j = (j + 1) % locale_grouping.length];
21282130
}
21292131
return t.reverse().join(locale_thousands);
21302132
} : d3_identity;
21312133
return function(specifier) {
2132-
var match = d3_format_re.exec(specifier), fill = match[1] || " ", align = match[2] || ">", sign = match[3] || "", symbol = match[4] || "", zfill = match[5], width = +match[6], comma = match[7], precision = match[8], type = match[9], scale = 1, prefix = "", suffix = "", integer = false;
2134+
var match = d3_format_re.exec(specifier), fill = match[1] || " ", align = match[2] || ">", sign = match[3] || "", symbol = match[4] || "", zfill = match[5], width = +match[6], comma = match[7], precision = match[8], type = match[9], scale = 1, prefix = "", suffix = "", integer = false, exponent = true;
21332135
if (precision) precision = +precision.substring(1);
21342136
if (zfill || fill === "0" && align === "=") {
21352137
zfill = fill = "0";
21362138
align = "=";
2137-
if (comma) width -= Math.floor((width - 1) / 4);
21382139
}
21392140
switch (type) {
21402141
case "n":
@@ -2161,6 +2162,8 @@
21612162
if (symbol === "#") prefix = "0" + type.toLowerCase();
21622163

21632164
case "c":
2165+
exponent = false;
2166+
21642167
case "d":
21652168
integer = true;
21662169
precision = 0;
@@ -2190,10 +2193,17 @@
21902193
value *= scale;
21912194
}
21922195
value = type(value, precision);
2193-
var i = value.lastIndexOf("."), before = i < 0 ? value : value.substring(0, i), after = i < 0 ? "" : locale_decimal + value.substring(i + 1);
2194-
if (!zfill && comma) before = formatGroup(before);
2196+
var i = value.lastIndexOf("."), before, after;
2197+
if (i < 0) {
2198+
var j = exponent ? value.lastIndexOf("e") : -1;
2199+
if (j < 0) before = value, after = ""; else before = value.substring(0, j), after = value.substring(j);
2200+
} else {
2201+
before = value.substring(0, i);
2202+
after = locale_decimal + value.substring(i + 1);
2203+
}
2204+
if (!zfill && comma) before = formatGroup(before, Infinity);
21952205
var length = prefix.length + before.length + after.length + (zcomma ? 0 : negative.length), padding = length < width ? new Array(length = width - length + 1).join(fill) : "";
2196-
if (zcomma) before = formatGroup(padding + before);
2206+
if (zcomma) before = formatGroup(padding + before, padding.length ? width - after.length : Infinity);
21972207
negative += prefix;
21982208
value = before + after;
21992209
return (align === "<" ? negative + value + padding : align === ">" ? padding + negative + value : align === "^" ? padding.substring(0, length >>= 1) + negative + value + padding.substring(length) : negative + (zcomma ? value : padding + value)) + fullSuffix;

‎d3.min.js‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/locale/number-format.js‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,16 @@ function d3_locale_numberFormat(locale) {
99
locale_thousands = locale.thousands,
1010
locale_grouping = locale.grouping,
1111
locale_currency = locale.currency,
12-
formatGroup = locale_grouping ? function(value) {
12+
formatGroup = locale_grouping && locale_thousands ? function(value, width) {
1313
var i = value.length,
1414
t = [],
1515
j = 0,
16-
g = locale_grouping[0];
17-
while (g > 0 && i > 0) {
16+
g = locale_grouping[0],
17+
length = 0;
18+
while (i > 0 && g > 0) {
19+
if (length + g + 1 > width) g = Math.max(1, width - length);
1820
t.push(value.substring(i -= g, i + g));
21+
if ((length += g + 1) > width) break;
1922
g = locale_grouping[j = (j + 1) % locale_grouping.length];
2023
}
2124
return t.reverse().join(locale_thousands);
@@ -35,14 +38,14 @@ function d3_locale_numberFormat(locale) {
3538
scale = 1,
3639
prefix = "",
3740
suffix = "",
38-
integer = false;
41+
integer = false,
42+
exponent = true;
3943

4044
if (precision) precision = +precision.substring(1);
4145

4246
if (zfill || fill === "0" && align === "=") {
4347
zfill = fill = "0";
4448
align = "=";
45-
if (comma) width -= Math.floor((width - 1) / 4);
4649
}
4750

4851
switch (type) {
@@ -53,7 +56,7 @@ function d3_locale_numberFormat(locale) {
5356
case "o":
5457
case "x":
5558
case "X": if (symbol === "#") prefix = "0" + type.toLowerCase();
56-
case "c":
59+
case "c": exponent = false;
5760
case "d": integer = true; precision = 0; break;
5861
case "s": scale = -1; type = "r"; break;
5962
}
@@ -97,17 +100,26 @@ function d3_locale_numberFormat(locale) {
97100

98101
// Break the value into the integer part (before) and decimal part (after).
99102
var i = value.lastIndexOf("."),
100-
before = i < 0 ? value : value.substring(0, i),
101-
after = i < 0 ? "" : locale_decimal + value.substring(i + 1);
103+
before,
104+
after;
105+
if (i < 0) {
106+
// If there is no decimal, break on "e" where appropriate.
107+
var j = exponent ? value.lastIndexOf("e") : -1;
108+
if (j < 0) before = value, after = "";
109+
else before = value.substring(0, j), after = value.substring(j);
110+
} else {
111+
before = value.substring(0, i);
112+
after = locale_decimal + value.substring(i + 1);
113+
}
102114

103-
// If the fill character is not "0", grouping is applied before padding.
104-
if (!zfill && comma) before = formatGroup(before);
115+
// If the fill character is not "0", grouping is applied before padding.
116+
if (!zfill && comma) before = formatGroup(before, Infinity);
105117

106118
var length = prefix.length + before.length + after.length + (zcomma ? 0 : negative.length),
107119
padding = length < width ? new Array(length = width - length + 1).join(fill) : "";
108120

109121
// If the fill character is "0", grouping is applied after padding.
110-
if (zcomma) before = formatGroup(padding + before);
122+
if (zcomma) before = formatGroup(padding + before, padding.length ? width - after.length : Infinity);
111123

112124
// Apply prefix.
113125
negative += prefix;

‎test/format/format-test.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,17 +214,24 @@ suite.addBatch({
214214
assert.strictEqual(f(-42), "-42");
215215
assert.strictEqual(f(-4200000), "-4,200,000");
216216
assert.strictEqual(f(-42000000), "-42,000,000");
217+
assert.strictEqual(f(1e21), "1e+21");
217218
},
218219
"can group thousands and zero fill": function(format) {
219220
assert.strictEqual(format("01,d")(0), "0");
220221
assert.strictEqual(format("01,d")(0), "0");
221222
assert.strictEqual(format("02,d")(0), "00");
222223
assert.strictEqual(format("03,d")(0), "000");
224+
assert.strictEqual(format("04,d")(0), "0,000");
223225
assert.strictEqual(format("05,d")(0), "0,000");
226+
assert.strictEqual(format("06,d")(0), "00,000");
224227
assert.strictEqual(format("08,d")(0), "0,000,000");
225228
assert.strictEqual(format("013,d")(0), "0,000,000,000");
226229
assert.strictEqual(format("021,d")(0), "0,000,000,000,000,000");
227230
assert.strictEqual(format("013,d")(-42000000), "-0,042,000,000");
231+
assert.strictEqual(format("012,d")(1e21), "0,000,001e+21");
232+
assert.strictEqual(format("013,d")(1e21), "0,000,001e+21");
233+
assert.strictEqual(format("014,d")(1e21), "00,000,001e+21");
234+
assert.strictEqual(format("015,d")(1e21), "000,000,001e+21");
228235
},
229236
"can group thousands and zero fill with overflow": function(format) {
230237
assert.strictEqual(format("01,d")(1), "1");
@@ -330,6 +337,8 @@ suite.addBatch({
330337
assert.strictEqual(format(">8,d")(0), " 0");
331338
assert.strictEqual(format(">13,d")(0), " 0");
332339
assert.strictEqual(format(">21,d")(0), " 0");
340+
assert.strictEqual(format(">21,d")(1000), " 1,000");
341+
assert.strictEqual(format(">21,d")(1e21), " 1e+21");
333342
},
334343
"align center": function(format) {
335344
assert.strictEqual(format("^1,d")(0), "0");
@@ -340,6 +349,8 @@ suite.addBatch({
340349
assert.strictEqual(format("^8,d")(0), " 0 ");
341350
assert.strictEqual(format("^13,d")(0), " 0 ");
342351
assert.strictEqual(format("^21,d")(0), " 0 ");
352+
assert.strictEqual(format("^21,d")(1000), " 1,000 ");
353+
assert.strictEqual(format("^21,d")(1e21), " 1e+21 ");
343354
},
344355
"pad after sign": function(format) {
345356
assert.strictEqual(format("=+1,d")(0), "+0");
@@ -350,6 +361,7 @@ suite.addBatch({
350361
assert.strictEqual(format("=+8,d")(0), "+ 0");
351362
assert.strictEqual(format("=+13,d")(0), "+ 0");
352363
assert.strictEqual(format("=+21,d")(0), "+ 0");
364+
assert.strictEqual(format("=+21,d")(1e21), "+ 1e+21");
353365
},
354366
"pad after sign with currency": function(format) {
355367
assert.strictEqual(format("=+$1,d")(0), "+$0");
@@ -360,6 +372,7 @@ suite.addBatch({
360372
assert.strictEqual(format("=+$8,d")(0), "+$ 0");
361373
assert.strictEqual(format("=+$13,d")(0), "+$ 0");
362374
assert.strictEqual(format("=+$21,d")(0), "+$ 0");
375+
assert.strictEqual(format("=+$21,d")(1e21), "+$ 1e+21");
363376
},
364377
"a space can denote positive numbers": function(format) {
365378
assert.strictEqual(format(" 1,d")(-1), "-1");
@@ -370,6 +383,7 @@ suite.addBatch({
370383
assert.strictEqual(format(" 8,d")(0), " 0");
371384
assert.strictEqual(format(" 13,d")(0), " 0");
372385
assert.strictEqual(format(" 21,d")(0), " 0");
386+
assert.strictEqual(format(" 21,d")(1e21), " 1e+21");
373387
},
374388
"can format negative zero": function(format) {
375389
assert.strictEqual(format("1d")(-0), "-0");
@@ -387,6 +401,18 @@ suite.addBatch({
387401
assert.strictEqual(f(-42), "-42");
388402
assert.strictEqual(f(-4200000), "-4,200,000");
389403
assert.strictEqual(f(-42000000), "-42,000,000");
404+
assert.strictEqual(f(1e21), "1e+21");
405+
},
406+
"\"n\" with zero padding": function(format) {
407+
assert.strictEqual(format("01n")(0), "0");
408+
assert.strictEqual(format("01n")(0), "0");
409+
assert.strictEqual(format("02n")(0), "00");
410+
assert.strictEqual(format("03n")(0), "000");
411+
assert.strictEqual(format("05n")(0), "0,000");
412+
assert.strictEqual(format("08n")(0), "0,000,000");
413+
assert.strictEqual(format("013n")(0), "0,000,000,000");
414+
assert.strictEqual(format("021n")(0), "0,000,000,000,000,000");
415+
assert.strictEqual(format("013n")(-42000000), "-0,042,000,000");
390416
},
391417
"unreasonable precision values are clamped to reasonable values": function(format) {
392418
assert.strictEqual(format(".30f")(0), "0.00000000000000000000");

0 commit comments

Comments
 (0)