On 2014-09-24 20:14, Xueming Shen wrote:
Shouldn't this one be at the same class (FormatSpecifier?) as the
"localizedMagnitudeExp"?
the webrev shows it is one level up.
Nice catch!
Btw, when we are here, maybe we can just remove the
FormatSpecifier.getZero(), it appears
we have a static version already at Formatter level.
They are not identical, and in fact, I think some subtle things would
change if localizedMagnitudeExp
isn't moved up: FormatSpecifier.getZero() check versus the locale the
Formatter was created
with while the static method compares against Locale.US. This changes
semantics when calling
format operations with null, which is a legal option:
Formatter f = new Formatter(Locale.X); // where Locale.X is a locale
where zero != '0'
f.format(null, "%.0e", 1000000000.0); // would print the exponent in
different ways
However, current behavior might actually be wrong since it's in
contradiction to the specification,
e.g., Formatter#format(Locale l, String format, Object ... args):
* @param l
* The {@linkplain java.util.Locale locale} to apply during
* formatting. If {@code l} is {@code null} then no
localization
* is applied. This does not change this object's locale
that was
* set during construction.
Most code interprets l == null like this, but FormatSpecifier.getZero()
is an exception. I think this
needs to be investigated and merits a bug of its own.
Updated localizedMagnitudeExp to reside in FormatSpecifier, leaving
getZero alone:
http://cr.openjdk.java.net/~redestad/8050142/webrev.10/
/Claes
On 09/24/2014 10:41 AM, Xueming Shen wrote:
Looks good.
On 09/23/2014 02:07 PM, Claes Redestad wrote:
How about:
// Specialized localization of exponents, where the source value
can only
// contain characters '0' through '9', starting at index offset,
and no
// group separators is added for any locale.
private void localizedMagnitudeExp(StringBuilder sb, char[] value,
final int offset, Locale l) {
char zero = getZero(l);
int len = value.length;
for (int j = offset; j < len; j++) {
char c = value[j];
sb.append((char) ((c - '0') + zero));
}
}
Webrev including this and the fixes for Conversion.SCIENTIFIC:
http://cr.openjdk.java.net/~redestad/8050142/webrev.09/
/Claes
On 2014-09-23 22:12, Xueming Shen wrote:
I don''t think an exponent should ever have a "dot', it always a
signed integer. I think we can
just remove the dead code, maybe put some wording to explain why no
group, no dot here.
-Sherman
On 09/23/2014 12:42 PM, Claes Redestad wrote:
Ouch... but wait... the char[] returned from
sun.misc.FormattedFloatingDecimal.getExponent() can never contain
a '.', so we'll never find a dot here. Remove the dead code or fix
the logic?
/Claes
On 2014-09-23 21:30, Xueming Shen wrote:
Also the logic in the loop of localizedMagnitudeExp() does not
look correct.
Shouldn't it be
char c= value[j];
if (c == '.') {
append l10n dot...
} else {
sb.append(c - '0' + zero);
}
it appears the 'else" is missing? or I read it wrong?
-Sherman
On 09/23/2014 12:27 PM, Claes Redestad wrote:
On 2014-09-23 21:14, Xueming Shen wrote:
On 09/22/2014 12:43 PM, Claes Redestad wrote:
Hi,
Sherman pointed out that there was a path that could actually
take a minor performance hit from this patch,
which would be unacceptable. This version takes the minimal
approach to addressing this by adding back a
method operating on a char[], simplified for the specific
usage case (the exponent part of a %g double formatting):
http://cr.openjdk.java.net/~redestad/8050142/webrev.07/
This latest patch passes using the extended test coverage of
java.util.Formatter I've proposed in 8058887, see
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-September/028849.html
Hi Claes,
Shouldn't we also keep the exp as char[] as well in "c ==
Conversion.SCIENTIFIC" case?
It appears the usage of exp is the same as the one in
"GENERAL", in which we are keeping
the simple char[] for exp.
You're right, of course. I'll update the webrev.
/Claes
