AuroraMiddleware/scuffed-code
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

ICU-8610 Responding to number skeleton code review feedback.

Browse Source 
X-SVN-Rev: 41483
This commit is contained in:
Shane Carr 2018-05-30 03:34:41 +00:00
parent 64a17225b5
commit 29937704cd
4 changed files with 44 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
icu4c/source/i18n/number_skeletons.cpp
View File

@ -407,25 +407,21 @@ enum_to_stem_string::decimalSeparatorDisplay(UNumberDecimalSeparatorDisplay valu
UnlocalizedNumberFormatter skeleton::create(const UnicodeString& skeletonString, UErrorCode& status) {
if (U_FAILURE(status)) { return {}; }
umtx_initOnce(gNumberSkeletonsInitOnce, &initNumberSkeletons, status);
if (U_FAILURE(status)) { return {}; }
MacroProps macros = parseSkeleton(skeletonString, status);
return NumberFormatter::with().macros(macros);
}
UnicodeString skeleton::generate(const MacroProps& macros, UErrorCode& status) {
if (U_FAILURE(status)) { return {}; }
umtx_initOnce(gNumberSkeletonsInitOnce, &initNumberSkeletons, status);
if (U_FAILURE(status)) { return {}; }
UnicodeString sb;
GeneratorHelpers::generateSkeleton(macros, sb, status);
return sb;
}
MacroProps skeleton::parseSkeleton(const UnicodeString& skeletonString, UErrorCode& status) {
if (U_FAILURE(status)) { return {}; }
// Add a trailing whitespace to the end of the skeleton string to make code cleaner.
UnicodeString tempSkeletonString(skeletonString);
tempSkeletonString.append(u' ');
@ -713,9 +709,15 @@ ParseState skeleton::parseOption(ParseState stem, const StringSegment& segment,
if (blueprint_helpers::parseExponentWidthOption(segment, macros, status)) {
return STATE_SCIENTIFIC;
}
if (U_FAILURE(status)) {
return {};
}
if (blueprint_helpers::parseExponentSignOption(segment, macros, status)) {
return STATE_SCIENTIFIC;
}
if (U_FAILURE(status)) {
return {};
}
break;
default:
break;
@ -727,6 +729,9 @@ ParseState skeleton::parseOption(ParseState stem, const StringSegment& segment,
if (blueprint_helpers::parseFracSigOption(segment, macros, status)) {
return STATE_NULL;
}
if (U_FAILURE(status)) {
return {};
}
break;
default:
break;
@ -739,6 +744,8 @@ ParseState skeleton::parseOption(ParseState stem, const StringSegment& segment,
}
void GeneratorHelpers::generateSkeleton(const MacroProps& macros, UnicodeString& sb, UErrorCode& status) {
if (U_FAILURE(status)) { return; }
// Supported options
if (GeneratorHelpers::notation(macros, sb, status)) {
sb.append(u' ');
@ -902,7 +909,7 @@ void blueprint_helpers::parseMeasureUnitOption(const StringSegment& segment, Mac
}
// Need to do char <-> UChar conversion...
if (U_FAILURE(status)) { return; }
U_ASSERT(U_SUCCESS(status));
CharString type;
SKELETON_UCHAR_TO_CHAR(type, stemString, 0, firstHyphen, status);
CharString subType;
@ -944,6 +951,7 @@ void blueprint_helpers::parseMeasurePerUnitOption(const StringSegment& segment,
// parsing code, put back the numerator unit, and put the new unit into per-unit.
MeasureUnit numerator = macros.unit;
parseMeasureUnitOption(segment, macros, status);
if (U_FAILURE(status)) { return; }
macros.perUnit = macros.unit;
macros.unit = numerator;
}
@ -1039,6 +1047,7 @@ blueprint_helpers::parseDigitsStem(const StringSegment& segment, MacroProps& mac
if (offset < segment.length()) {
// throw new SkeletonSyntaxException("Invalid significant digits stem", segment);
status = U_NUMBER_SKELETON_SYNTAX_ERROR;
return;
}
// Use the public APIs to enforce bounds checking
if (maxSig == -1) {
@ -1121,6 +1130,7 @@ bool blueprint_helpers::parseFracSigOption(const StringSegment& segment, MacroPr
void blueprint_helpers::parseIncrementOption(const StringSegment& segment, MacroProps& macros,
UErrorCode& status) {
// Need to do char <-> UChar conversion...
U_ASSERT(U_SUCCESS(status));
CharString buffer;
SKELETON_UCHAR_TO_CHAR(buffer, segment.toTempUnicodeString(), 0, segment.length(), status);
@ -1218,11 +1228,13 @@ void blueprint_helpers::generateIntegerWidthOption(int32_t minInt, int32_t maxIn
void blueprint_helpers::parseNumberingSystemOption(const StringSegment& segment, MacroProps& macros,
UErrorCode& status) {
// Need to do char <-> UChar conversion...
U_ASSERT(U_SUCCESS(status));
CharString buffer;
SKELETON_UCHAR_TO_CHAR(buffer, segment.toTempUnicodeString(), 0, segment.length(), status);
NumberingSystem* ns = NumberingSystem::createInstanceByName(buffer.data(), status);
if (ns == nullptr) {
if (ns == nullptr || U_FAILURE(status)) {
// This is a skeleton syntax error; don't bubble up the low-level NumberingSystem error
// throw new SkeletonSyntaxException("Unknown numbering system", segment);
status = U_NUMBER_SKELETON_SYNTAX_ERROR;
return;
@ -1239,6 +1251,7 @@ void blueprint_helpers::generateNumberingSystemOption(const NumberingSystem& ns,
void blueprint_helpers::parseScaleOption(const StringSegment& segment, MacroProps& macros,
UErrorCode& status) {
// Need to do char <-> UChar conversion...
U_ASSERT(U_SUCCESS(status));
CharString buffer;
SKELETON_UCHAR_TO_CHAR(buffer, segment.toTempUnicodeString(), 0, segment.length(), status);
@ -1246,6 +1259,7 @@ void blueprint_helpers::parseScaleOption(const StringSegment& segment, MacroProp
if (U_FAILURE(status)) { return; }
decnum->setTo({buffer.data(), buffer.length()}, status);
if (U_FAILURE(status)) {
// This is a skeleton syntax error; don't let the low-level decnum error bubble up
status = U_NUMBER_SKELETON_SYNTAX_ERROR;
return;
}
@ -1254,13 +1268,13 @@ void blueprint_helpers::parseScaleOption(const StringSegment& segment, MacroProp
macros.scale = {0, decnum.orphan()};
}
void
blueprint_helpers::generateScaleOption(int32_t magnitude, const DecNum* arbitrary, UnicodeString& sb,
void blueprint_helpers::generateScaleOption(int32_t magnitude, const DecNum* arbitrary, UnicodeString& sb,
UErrorCode& status) {
// Utilize DecimalQuantity/double_conversion to format this for us.
DecimalQuantity dq;
if (arbitrary != nullptr) {
dq.setToDecNum(*arbitrary, status);
if (U_FAILURE(status)) { return; }
} else {
dq.setToInt(1);
}
@ -1295,6 +1309,9 @@ bool GeneratorHelpers::notation(const MacroProps& macros, UnicodeString& sb, UEr
if (impl.fMinExponentDigits > 1) {
sb.append(u'/');
blueprint_helpers::generateExponentWidthOption(impl.fMinExponentDigits, sb, status);
if (U_FAILURE(status)) {
return false;
}
}
if (impl.fExponentSignDisplay != UNUM_SIGN_AUTO) {
sb.append(u'/');
@ -1310,7 +1327,11 @@ bool GeneratorHelpers::notation(const MacroProps& macros, UnicodeString& sb, UEr
bool GeneratorHelpers::unit(const MacroProps& macros, UnicodeString& sb, UErrorCode& status) {
if (utils::unitIsCurrency(macros.unit)) {
sb.append(u"currency/", -1);
blueprint_helpers::generateCurrencyOption({macros.unit, status}, sb, status);
CurrencyUnit currency(macros.unit, status);
if (U_FAILURE(status)) {
return false;
}
blueprint_helpers::generateCurrencyOption(currency, sb, status);
return true;
} else if (utils::unitIsNoUnit(macros.unit)) {
if (utils::unitIsPercent(macros.unit)) {

2
icu4c/source/i18n/number_skeletons.h
View File

@ -141,7 +141,7 @@ UnicodeString generate(const MacroProps& macros, UErrorCode& status);
MacroProps parseSkeleton(const UnicodeString& skeletonString, UErrorCode& status);
/**
* Given that the current segment represents an stem, parse it and save the result.
* Given that the current segment represents a stem, parse it and save the result.
*
* @return The next state after parsing this stem, corresponding to what subset of options to expect.
*/

15
icu4c/source/test/intltest/numbertest_skeletons.cpp
View File

@ -31,6 +31,8 @@ void NumberSkeletonTest::runIndexedTest(int32_t index, UBool exec, const char*&
}
void NumberSkeletonTest::validTokens() {
IcuTestErrorCode status(*this, "validTokens");
// This tests only if the tokens are valid, not their behavior.
// Most of these are from the design doc.
static const char16_t* cases[] = {
@ -105,9 +107,10 @@ void NumberSkeletonTest::validTokens() {
for (auto& cas : cases) {
UnicodeString skeletonString(cas);
UErrorCode status = U_ZERO_ERROR;
status.setScope(skeletonString);
NumberFormatter::forSkeleton(skeletonString, status);
assertSuccess(skeletonString, status);
status.errIfFailureAndReset();
}
}
@ -144,7 +147,7 @@ void NumberSkeletonTest::invalidTokens() {
u"integer-width/+0#",
u"scientific/foo"};
expectedErrorSkeleton(cases, sizeof(cases) / sizeof(*cases));
expectedErrorSkeleton(cases, UPRV_LENGTHOF(cases));
}
void NumberSkeletonTest::unknownTokens() {
@ -157,7 +160,7 @@ void NumberSkeletonTest::unknownTokens() {
u"numbering-system/français", // non-invariant characters for C++
u"currency-USD"};
expectedErrorSkeleton(cases, sizeof(cases) / sizeof(*cases));
expectedErrorSkeleton(cases, UPRV_LENGTHOF(cases));
}
void NumberSkeletonTest::unexpectedTokens() {
@ -168,7 +171,7 @@ void NumberSkeletonTest::unexpectedTokens() {
u"precision-integer/ group-off",
u"precision-integer// group-off"};
expectedErrorSkeleton(cases, sizeof(cases) / sizeof(*cases));
expectedErrorSkeleton(cases, UPRV_LENGTHOF(cases));
}
void NumberSkeletonTest::duplicateValues() {
@ -181,7 +184,7 @@ void NumberSkeletonTest::duplicateValues() {
u"engineering compact-long",
u"sign-auto sign-always"};
expectedErrorSkeleton(cases, sizeof(cases) / sizeof(*cases));
expectedErrorSkeleton(cases, UPRV_LENGTHOF(cases));
}
void NumberSkeletonTest::stemsRequiringOption() {
@ -224,6 +227,7 @@ void NumberSkeletonTest::defaultTokens() {
skeletonString, status).toSkeleton(status);
// Skeleton should become empty when normalized
assertEquals(skeletonString, u"", normalized);
status.errIfFailureAndReset();
}
}
@ -246,6 +250,7 @@ void NumberSkeletonTest::flexibleSeparators() {
.formatDouble(5142.3, status)
.toString();
assertEquals(skeletonString, expected, actual);
status.errIfFailureAndReset();
}
}

2
icu4j/main/classes/core/src/com/ibm/icu/number/NumberSkeletonImpl.java
View File

@ -589,7 +589,7 @@ class NumberSkeletonImpl {
}
/**
* Given that the current segment represents an stem, parse it and save the result.
* Given that the current segment represents a stem, parse it and save the result.
*
* @return The next state after parsing this stem, corresponding to what subset of options to expect.
*/