-
Notifications
You must be signed in to change notification settings - Fork 412
CLDR-18963 Explicit numberSystem attribute #5149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1441,6 +1441,8 @@ CLDR data files are interpreted according to the LDML specification (http://unic | |
| <!--@DEPRECATED:true, false--> | ||
| <!ATTLIST pattern references CDATA #IMPLIED > | ||
| <!--@METADATA--> | ||
| <!ATTLIST pattern numberSystem CDATA #IMPLIED > | ||
| <!--@MATCH:bcp47/nu--> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| <!ELEMENT datetimeSkeleton ( #PCDATA ) > | ||
| <!ATTLIST datetimeSkeleton numbers CDATA #IMPLIED > | ||
|
|
@@ -2062,7 +2064,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic | |
| <!ATTLIST decimal references CDATA #IMPLIED > | ||
| <!--@METADATA--> | ||
| <!ATTLIST decimal numberSystem CDATA #IMPLIED > | ||
| <!--@DEPRECATED--> | ||
| <!--@MATCH:bcp47/nu--> | ||
|
|
||
| <!ELEMENT group ( #PCDATA ) > | ||
| <!ATTLIST group alt NMTOKENS #IMPLIED > | ||
|
|
@@ -2073,7 +2075,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic | |
| <!ATTLIST group references CDATA #IMPLIED > | ||
| <!--@METADATA--> | ||
| <!ATTLIST group numberSystem CDATA #IMPLIED > | ||
| <!--@DEPRECATED--> | ||
| <!--@MATCH:bcp47/nu--> | ||
|
|
||
| <!ELEMENT list ( #PCDATA ) > | ||
| <!ATTLIST list alt NMTOKENS #IMPLIED > | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a failure caused either by this addition, or by one of the later changes in this file where
numberSystemgets changed from@DEPRECATEDto@MATCH:bcp47/nu. The failure is:I've tried moving
...pattern numberSystem...up before...pattern references...or before...pattern draft...but similar errors occur. The reference tovalidSubLocalesin the error messages is especially mysterious. I'll continue investigation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Experimentally I tried adding items for
pattern,decimal, andgrouptoattributeOrderGrandfathered, 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 forsymbolswas already present inattributeOrderGrandfathered. This leads me to suspect that no other solution exists; otherwise why was the item forsymbolsalready 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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also do not understand what that ordering test is about, but the rest of the PR looks good to me.