From: Shane Carr Date: Fri, 15 Feb 2019 10:41:32 +0000 (-0800) Subject: ICU-20144 Implementing numsys-dependent range pattern loading. X-Git-Tag: release-64-rc~68 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9cdb660b5e0dabeab70c7c10ac3ad29ec2ffce03;p=icu ICU-20144 Implementing numsys-dependent range pattern loading. --- diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp index de6ec65486b..a7f162ecc63 100644 --- a/icu4c/source/i18n/number_formatimpl.cpp +++ b/icu4c/source/i18n/number_formatimpl.cpp @@ -207,6 +207,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, nsLocal.adoptInstead(ns); } const char* nsName = U_SUCCESS(status) ? ns->getName() : "latn"; + uprv_strncpy(fMicros.nsName, nsName, 8); + fMicros.nsName[8] = 0; // guarantee NUL-terminated // Resolve the symbols. Do this here because currency may need to customize them. if (macros.symbols.isDecimalFormatSymbols()) { diff --git a/icu4c/source/i18n/number_formatimpl.h b/icu4c/source/i18n/number_formatimpl.h index fda38c92845..fd8708c532e 100644 --- a/icu4c/source/i18n/number_formatimpl.h +++ b/icu4c/source/i18n/number_formatimpl.h @@ -64,6 +64,10 @@ class NumberFormatterImpl : public UMemory { int32_t getPrefixSuffix(int8_t signum, StandardPlural::Form plural, NumberStringBuilder& outString, UErrorCode& status) const; + const MicroProps& getRawMicroProps() const { + return fMicros; + } + /** * Synthesizes the output string from a MicroProps and DecimalQuantity. * This method formats only the main number, not affixes. diff --git a/icu4c/source/i18n/number_microprops.h b/icu4c/source/i18n/number_microprops.h index daa887bb0dd..d2393aea50c 100644 --- a/icu4c/source/i18n/number_microprops.h +++ b/icu4c/source/i18n/number_microprops.h @@ -32,6 +32,7 @@ struct MicroProps : public MicroPropsGenerator { UNumberSignDisplay sign; UNumberDecimalSeparatorDisplay decimal; bool useCurrency; + char nsName[9]; // Note: This struct has no direct ownership of the following pointers. const DecimalFormatSymbols* symbols; diff --git a/icu4c/source/i18n/numrange_impl.cpp b/icu4c/source/i18n/numrange_impl.cpp index 0cffb05628d..05eb2b84de3 100644 --- a/icu4c/source/i18n/numrange_impl.cpp +++ b/icu4c/source/i18n/numrange_impl.cpp @@ -41,12 +41,12 @@ class NumberRangeDataSink : public ResourceSink { if (U_FAILURE(status)) { return; } for (int i = 0; miscTable.getKeyAndValue(i, key, value); i++) { if (uprv_strcmp(key, "range") == 0) { - if (fData.rangePattern.getArgumentLimit() != 0) { + if (hasRangeData()) { continue; // have already seen this pattern } fData.rangePattern = {value.getUnicodeString(status), status}; } else if (uprv_strcmp(key, "approximately") == 0) { - if (fData.approximatelyPattern.getArgumentLimit() != 0) { + if (hasApproxData()) { continue; // have already seen this pattern } fData.approximatelyPattern = {value.getUnicodeString(status), status}; @@ -54,6 +54,27 @@ class NumberRangeDataSink : public ResourceSink { } } + bool hasRangeData() { + return fData.rangePattern.getArgumentLimit() != 0; + } + + bool hasApproxData() { + return fData.approximatelyPattern.getArgumentLimit() != 0; + } + + bool isComplete() { + return hasRangeData() && hasApproxData(); + } + + void fillInDefaults(UErrorCode& status) { + if (!hasRangeData()) { + fData.rangePattern = {u"{0}–{1}", status}; + } + if (!hasApproxData()) { + fData.approximatelyPattern = {u"~{0}", status}; + } + } + private: NumberRangeData& fData; }; @@ -68,19 +89,21 @@ void getNumberRangeData(const char* localeName, const char* nsName, NumberRangeD dataPath.append("NumberElements/", -1, status); dataPath.append(nsName, -1, status); dataPath.append("/miscPatterns", -1, status); - ures_getAllItemsWithFallback(rb.getAlias(), dataPath.data(), sink, status); if (U_FAILURE(status)) { return; } - // TODO: Is it necessary to manually fall back to latn, or does the data sink take care of that? - - if (data.rangePattern.getArgumentLimit() == 0) { - // No data! - data.rangePattern = {u"{0}–{1}", status}; + UErrorCode localStatus = U_ZERO_ERROR; + ures_getAllItemsWithFallback(rb.getAlias(), dataPath.data(), sink, localStatus); + if (U_FAILURE(localStatus) && localStatus != U_MISSING_RESOURCE_ERROR) { + status = localStatus; + return; } - if (data.approximatelyPattern.getArgumentLimit() == 0) { - // No data! - data.approximatelyPattern = {u"~{0}", status}; + + // Fall back to latn if necessary + if (!sink.isComplete()) { + ures_getAllItemsWithFallback(rb.getAlias(), "NumberElements/latn/miscPatterns", sink, status); } + + sink.fillInDefaults(status); } class PluralRangesDataSink : public ResourceSink { @@ -177,15 +200,14 @@ NumberRangeFormatterImpl::NumberRangeFormatterImpl(const RangeMacroProps& macros fCollapse(macros.collapse), fIdentityFallback(macros.identityFallback) { - // TODO: As of this writing (ICU 63), there is no locale that has different number miscPatterns - // based on numbering system. Therefore, data is loaded only from latn. If this changes, - // this part of the code should be updated to load from the local numbering system. - // The numbering system could come from the one specified in the NumberFormatter passed to - // numberFormatterBoth() or similar. - // See ICU-20144 + const char* nsName = formatterImpl1.getRawMicroProps().nsName; + if (uprv_strcmp(nsName, formatterImpl2.getRawMicroProps().nsName) != 0) { + status = U_ILLEGAL_ARGUMENT_ERROR; + return; + } NumberRangeData data; - getNumberRangeData(macros.locale.getName(), "latn", data, status); + getNumberRangeData(macros.locale.getName(), nsName, data, status); if (U_FAILURE(status)) { return; } fRangeFormatter = data.rangePattern; fApproximatelyModifier = {data.approximatelyPattern, UNUM_FIELD_COUNT, false}; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java index 6aeda32775e..2fce2018996 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java @@ -12,6 +12,7 @@ public class MicroProps implements Cloneable, MicroPropsGenerator { // Populated globally: public SignDisplay sign; public DecimalFormatSymbols symbols; + public String nsName; public Padder padding; public DecimalSeparatorDisplay decimal; public IntegerWidth integerWidth; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java index 423bc1c346d..b6b74f8e3ed 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java @@ -44,7 +44,8 @@ class NumberFormatterImpl { /** Builds a "safe" MicroPropsGenerator, which is thread-safe and can be used repeatedly. */ public NumberFormatterImpl(MacroProps macros) { - this(macrosToMicroGenerator(macros, true)); + micros = new MicroProps(true); + microPropsGenerator = macrosToMicroGenerator(macros, micros, true); } /** @@ -71,18 +72,16 @@ class NumberFormatterImpl { byte signum, StandardPlural plural, NumberStringBuilder output) { - MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, false); + MicroProps micros = new MicroProps(false); + MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, micros, false); return getPrefixSuffixImpl(microPropsGenerator, signum, output); } private static final Currency DEFAULT_CURRENCY = Currency.getInstance("XXX"); + final MicroProps micros; final MicroPropsGenerator microPropsGenerator; - private NumberFormatterImpl(MicroPropsGenerator microPropsGenerator) { - this.microPropsGenerator = microPropsGenerator; - } - /** * Evaluates the "safe" MicroPropsGenerator created by "fromMacros". */ @@ -109,8 +108,9 @@ class NumberFormatterImpl { } private static MicroProps preProcessUnsafe(MacroProps macros, DecimalQuantity inValue) { - MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, false); - MicroProps micros = microPropsGenerator.processQuantity(inValue); + MicroProps micros = new MicroProps(false); + MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, micros, false); + micros = microPropsGenerator.processQuantity(inValue); micros.rounder.apply(inValue); if (micros.integerWidth.maxInt == -1) { inValue.setMinInteger(micros.integerWidth.minInt); @@ -138,6 +138,10 @@ class NumberFormatterImpl { return micros.modMiddle.getPrefixLength(); } + public MicroProps getRawMicroProps() { + return micros; + } + ////////// private static boolean unitIsCurrency(MeasureUnit unit) { @@ -172,8 +176,7 @@ class NumberFormatterImpl { * value will not be thread-safe, intended for a single "one-shot" use only. * Building the thread-safe object is more expensive. */ - private static MicroPropsGenerator macrosToMicroGenerator(MacroProps macros, boolean safe) { - MicroProps micros = new MicroProps(safe); + private static MicroPropsGenerator macrosToMicroGenerator(MacroProps macros, MicroProps micros, boolean safe) { MicroPropsGenerator chain = micros; // TODO: Normalize the currency (accept symbols from DecimalFormatSymbols)? @@ -203,7 +206,7 @@ class NumberFormatterImpl { // TODO: Is there a way to avoid creating the NumberingSystem object? ns = NumberingSystem.getInstance(macros.loc); } - String nsName = ns.getName(); + micros.nsName = ns.getName(); // Resolve the symbols. Do this here because currency may need to customize them. if (macros.symbols instanceof DecimalFormatSymbols) { @@ -240,7 +243,7 @@ class NumberFormatterImpl { patternStyle = NumberFormat.CURRENCYSTYLE; } pattern = NumberFormat - .getPatternForStyleAndNumberingSystem(macros.loc, nsName, patternStyle); + .getPatternForStyleAndNumberingSystem(macros.loc, micros.nsName, patternStyle); } ParsedPatternInfo patternInfo = PatternStringParser.parseToPatternInfo(pattern); @@ -372,7 +375,7 @@ class NumberFormatterImpl { && macros.unitWidth != UnitWidth.FULL_NAME) ? CompactType.CURRENCY : CompactType.DECIMAL; chain = ((CompactNotation) macros.notation).withLocaleData(macros.loc, - nsName, + micros.nsName, compactType, rules, safe ? patternMod : null, diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java index b1405fe80ef..8b9059bbcea 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java @@ -2,6 +2,8 @@ // License & terms of use: http://www.unicode.org/copyright.html#License package com.ibm.icu.number; +import java.util.MissingResourceException; + import com.ibm.icu.impl.ICUData; import com.ibm.icu.impl.ICUResourceBundle; import com.ibm.icu.impl.PatternProps; @@ -65,16 +67,37 @@ class NumberRangeFormatterImpl { public void put(UResource.Key key, UResource.Value value, boolean noFallback) { UResource.Table miscTable = value.getTable(); for (int i = 0; miscTable.getKeyAndValue(i, key, value); ++i) { - if (key.contentEquals("range") && rangePattern == null) { + if (key.contentEquals("range") && !hasRangeData()) { String pattern = value.getString(); rangePattern = SimpleFormatterImpl.compileToStringMinMaxArguments(pattern, sb, 2, 2); } - if (key.contentEquals("approximately") && approximatelyPattern == null) { + if (key.contentEquals("approximately") && !hasApproxData()) { String pattern = value.getString(); approximatelyPattern = SimpleFormatterImpl.compileToStringMinMaxArguments(pattern, sb, 1, 1); // 1 arg, as in "~{0}" } } } + + private boolean hasRangeData() { + return rangePattern != null; + } + + private boolean hasApproxData() { + return approximatelyPattern != null; + } + + public boolean isComplete() { + return hasRangeData() && hasApproxData(); + } + + public void fillInDefaults() { + if (!hasRangeData()) { + rangePattern = SimpleFormatterImpl.compileToStringMinMaxArguments("{0}–{1}", sb, 2, 2); + } + if (!hasApproxData()) { + approximatelyPattern = SimpleFormatterImpl.compileToStringMinMaxArguments("~{0}", sb, 1, 1); + } + } } private static void getNumberRangeData( @@ -89,17 +112,19 @@ class NumberRangeFormatterImpl { sb.append(nsName); sb.append("/miscPatterns"); String key = sb.toString(); - resource.getAllItemsWithFallback(key, sink); - - // TODO: Is it necessary to manually fall back to latn, or does the data sink take care of that? - - if (sink.rangePattern == null) { - sink.rangePattern = SimpleFormatterImpl.compileToStringMinMaxArguments("{0}–{1}", sb, 2, 2); + try { + resource.getAllItemsWithFallback(key, sink); + } catch (MissingResourceException e) { + // ignore; fall back to latn } - if (sink.approximatelyPattern == null) { - sink.approximatelyPattern = SimpleFormatterImpl.compileToStringMinMaxArguments("~{0}", sb, 1, 1); + + // Fall back to latn if necessary + if (!sink.isComplete()) { + resource.getAllItemsWithFallback("NumberElements/latn/miscPatterns", sink); } + sink.fillInDefaults(); + out.fRangePattern = sink.rangePattern; out.fApproximatelyModifier = new SimpleModifier(sink.approximatelyPattern, null, false); } @@ -116,14 +141,11 @@ class NumberRangeFormatterImpl { fIdentityFallback = macros.identityFallback != null ? macros.identityFallback : NumberRangeFormatter.RangeIdentityFallback.APPROXIMATELY; - // TODO: As of this writing (ICU 63), there is no locale that has different number miscPatterns - // based on numbering system. Therefore, data is loaded only from latn. If this changes, - // this part of the code should be updated to load from the local numbering system. - // The numbering system could come from the one specified in the NumberFormatter passed to - // numberFormatterBoth() or similar. - // See ICU-20144 - - getNumberRangeData(macros.loc, "latn", this); + String nsName = formatterImpl1.getRawMicroProps().nsName; + if (nsName == null || !nsName.equals(formatterImpl2.getRawMicroProps().nsName)) { + throw new IllegalArgumentException("Both formatters must have same numbering system"); + } + getNumberRangeData(macros.loc, nsName, this); // TODO: Get locale from PluralRules instead? fPluralRanges = new StandardPluralRanges(macros.loc);