Code review feedback incorporated.

This commit is contained in:
Hugo van der Merwe 2020-07-16 02:23:50 +02:00
parent c26a0e244f
commit ac513d70ce
4 changed files with 8 additions and 11 deletions

View file

@ -1006,8 +1006,6 @@ void blueprint_helpers::parseMeasureUnitOption(const StringSegment& segment, Mac
static constexpr int32_t CAPACITY = 30;
MeasureUnit units[CAPACITY];
UErrorCode localStatus = U_ZERO_ERROR;
// TODO(units): revisit this: deals with hard-coded set of measure units?
// Can it handle complex units correctly?
int32_t numUnits = MeasureUnit::getAvailable(type.data(), units, CAPACITY, localStatus);
if (U_FAILURE(localStatus)) {
// More than 30 units in this type?
@ -1574,7 +1572,7 @@ bool GeneratorHelpers::perUnit(const MacroProps& macros, UnicodeString& sb, UErr
bool GeneratorHelpers::usage(const MacroProps& macros, UnicodeString& sb, UErrorCode& status) {
if (macros.usage.fLength > 0) {
sb.append(u"usage/", -1);
sb.append(UnicodeString(macros.usage.fUsage));
sb.append(UnicodeString(macros.usage.fUsage, -1, US_INV));
return true;
}
return false;

View file

@ -48,7 +48,7 @@ enum ParseState {
STATE_INCREMENT_PRECISION,
STATE_MEASURE_UNIT,
STATE_PER_MEASURE_UNIT,
STATE_IDENTIFIER_UNIT, // A MeasureUnit Identifier. TODO(units): Unused?
STATE_IDENTIFIER_UNIT,
STATE_UNIT_USAGE,
STATE_CURRENCY_UNIT,
STATE_INTEGER_WIDTH,

View file

@ -329,10 +329,9 @@ int32_t getPreferenceMetadataIndex(const MaybeStackVector<UnitPreferenceMetadata
} else if (uprv_strcmp(desired.usage.data(), "default") != 0) {
desired.usage.truncate(0).append("default", status);
} else {
// TODO(units): we fail with U_MISSING_RESOURCE_ERROR for an invalid
// usage. For an unrecognised measure-unit, skeleton parsing would
// result in U_NUMBER_SKELETON_SYNTAX_ERROR instead. Do we want to
// be smarter about recognised usages? (Build a trie? :-)
// TODO(icu-units/icu#36): reconsider consistency of errors.
// Currently this U_MISSING_RESOURCE_ERROR propagates when a
// U_NUMBER_SKELETON_SYNTAX_ERROR might be much more intuitive.
status = U_MISSING_RESOURCE_ERROR;
return -1;
}

View file

@ -689,7 +689,7 @@ void NumberFormatterApiTest::unitUsage() {
assertEquals("unitUsage() en-ZA road", "300 m", formattedNum.toString(status));
assertFormatDescendingBig(
u"unitUsage() en-ZA road",
u"measure-unit/length-meter usage/road", // Alternatives: usage/length-road, unit-usage/road? FIXME
u"measure-unit/length-meter usage/road",
u"unit/meter usage/road",
unloc_formatter,
Locale("en-ZA"),
@ -711,7 +711,7 @@ void NumberFormatterApiTest::unitUsage() {
assertEquals("unitUsage() en-GB road", "328 yd", formattedNum.toString(status));
assertFormatDescendingBig(
u"unitUsage() en-GB road",
u"measure-unit/length-meter usage/road", // Alternatives: usage/length-road, unit-usage/road? FIXME
u"measure-unit/length-meter usage/road",
u"unit/meter usage/road",
unloc_formatter,
Locale("en-GB"),
@ -733,7 +733,7 @@ void NumberFormatterApiTest::unitUsage() {
assertEquals("unitUsage() en-US road", "984 ft", formattedNum.toString(status));
assertFormatDescendingBig(
u"unitUsage() en-US road",
u"measure-unit/length-meter usage/road", // Alternatives: usage/length-road, unit-usage/road? FIXME
u"measure-unit/length-meter usage/road",
u"unit/meter usage/road",
unloc_formatter,
Locale("en-US"),