CLDR-18963 Explicit numberSystem attribute #5149
Conversation
-Revise ldml.dtd to allow numberSystem attribute for pattern element -Add numberSystem (latn) for pattern/decimal/group elements where needed in locales ca, el, en_150, en_AT, eu, gl, it, tr -Re-enable the check for numberSystem in CheckNumbers.java -Use parts.containsAttribute instead of parts.getAttribute(2, ...) since the element index may not be 2
|
This PR replaces #4974 It uses the v49 ticket number instead of the v48 ticket number. It does not change the "test" files named ldml.dtd. There is one question by @macchiati in #4974 that I replied to in that PR: "I take it that NumericType still gets these items, so they aren't skipped by line 258.". I don't know whether it requires further attention... |
-Revise ldml.dtd to allow numberSystem attribute for pattern element -Add numberSystem (latn) for pattern/decimal/group elements where needed in locales ca, el, en_150, en_AT, eu, gl, it, tr -Re-enable the check for numberSystem in CheckNumbers.java -Use parts.containsAttribute instead of parts.getAttribute(2, ...) since the element index may not be 2
| <!ATTLIST pattern references CDATA #IMPLIED > | ||
| <!--@METADATA--> | ||
| <!ATTLIST pattern numberSystem CDATA #IMPLIED > | ||
| <!--@MATCH:bcp47/nu--> |
There was a problem hiding this comment.
There's a failure caused either by this addition, or by one of the later changes in this file where numberSystem gets changed from @DEPRECATED to @MATCH:bcp47/nu. The failure is:
Caused by: java.lang.IllegalArgumentException: Inconsistent requested ordering: cannot merge if we have [...A...B...] and [...B...A...]: {references=[draft, references], standard=[draft, standard, references, validSubLocales], validSubLocales=[draft, standard, references, validSubLocales], numberSystem=[draft, validSubLocales, numberSystem], draft=[numberSystem, draft, references]}
at org.unicode.cldr.util.MergeLists.merge(MergeLists.java:68)
at org.unicode.cldr.util.DtdData.freeze(DtdData.java:974)
I've tried moving ...pattern numberSystem... up before ...pattern references... or before ...pattern draft... but similar errors occur. The reference to validSubLocales in the error messages is especially mysterious. I'll continue investigation.
There was a problem hiding this comment.
Experimentally I tried adding items for pattern, decimal, and group to attributeOrderGrandfathered, and that succeeded in making the test pass. I expected this would only be a temporary work-around. Then I noticed that a completely analogous item for symbols was already present in attributeOrderGrandfathered. This leads me to suspect that no other solution exists; otherwise why was the item for symbols already present?
However, I still don't fully understand the test and what "ordering" rules it is enforcing, so I hope for a review by someone who understands it better, rather than a rubber stamp.
There was a problem hiding this comment.
I also do not understand what that ordering test is about, but the rest of the PR looks good to me.
-Revise ldml.dtd to allow numberSystem attribute for pattern element -Add numberSystem (latn) for pattern/decimal/group elements where needed in locales ca, el, en_150, en_AT, eu, gl, it, tr -Re-enable the check for numberSystem in CheckNumbers.java -Use parts.containsAttribute instead of parts.getAttribute(2, ...) since the element index may not be 2
-Change ❰NBTSP❱ to U+202F NARROW NO-BREAK SPACE (as had evidently been done in an earlier commit; this would have been a reversion)
| <!ATTLIST pattern references CDATA #IMPLIED > | ||
| <!--@METADATA--> | ||
| <!ATTLIST pattern numberSystem CDATA #IMPLIED > | ||
| <!--@MATCH:bcp47/nu--> |
There was a problem hiding this comment.
I guess this does have to be #IMPLIED and not #REQUIRED because this is an existing element for which we did not formerly require a numberSystem attribute. Sorry for my mistaken earlier comment on this.
There was a problem hiding this comment.
Yes, I tried changing it to REQUIRED and encountered errors with other kinds of path where it's still optional. I guess the test wouldn't be needed if it were always required.
2 participants
-Revise ldml.dtd to allow numberSystem attribute for pattern element
-Add numberSystem (latn) for pattern/decimal/group elements where needed in locales ca, el, en_150, en_AT, eu, gl, it, tr
-Re-enable the check for numberSystem in CheckNumbers.java
-Use parts.containsAttribute instead of parts.getAttribute(2, ...) since the element index may not be 2
-Add three items to attributeOrderGrandfathered in TestDtdData.java: like the existing //ldml…/symbols, add //ldml…/pattern, //ldml…/decimal, and //ldml…/group, in order for the test to pass
CLDR-18963
ALLOW_MANY_COMMITS=true