-
Notifications
You must be signed in to change notification settings - Fork 422
CLDR-19157 Restrict era codes to 8 characters #5283
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?
Conversation
c2cef2b to
0cbe2bf
Compare
This comment was marked as spam.
This comment was marked as spam.
0cbe2bf to
2a547cc
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
2a547cc to
ae0055d
Compare
This comment was marked as spam.
This comment was marked as spam.
ae0055d to
f16ca0f
Compare
This comment was marked as spam.
This comment was marked as spam.
f16ca0f to
76d8510
Compare
This comment was marked as spam.
This comment was marked as spam.
76d8510 to
3193c35
Compare
This comment was marked as spam.
This comment was marked as spam.
3193c35 to
df28db3
Compare
This comment was marked as spam.
This comment was marked as spam.
macchiati
left a comment
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 don't see a change to the spec that limits the length to 8 (for 48 and beyond)
- I think you said that 48 got rid of the ones that were longer, if different then adjust.
|
what exactly do you mean? |
| final CalendarData c = cal.get(calType); | ||
| for (final Integer n : c) { | ||
| if (c.get(n).getCode() != null) { | ||
| assertTrue(c.get(n).getCode().length() <= 8); |
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.
Suggested change
assertTrue(c.get(n).getCode().length() <= 8);
final String code = c.get(n).getCode();
assertTrue(code.length() <= 8, () -> "code " + code + " > 8");
otherwise it will show "assertion failed: false", but with this it will fail with "long era code: abcdefghij > 8"
srl295
left a comment
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.
Looks good, with a suggestion for a better err message
| Each `era` element has a `code` attribute and optional `aliases` attributes that define invariant strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below). | ||
| The `code` is unique within the calendar, and the `aliases` are space-separated identifiers, each also unique within the calendar. | ||
| Each `era` element has a `code` attribute and optional `aliases` attributes that define stable strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below). | ||
| The `code` defines the primary identifier for the era, and `aliases` are space-separated additional identifiers. All identifiers are valid BCP-47 subtags, i.e. 2-8 ASCII alphanumeric characters. |
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.
This says that each subtag is 2-8 characters. There_ is no statement that the total length of the code (including all subtags + separators) is limited to 8 characters, as per:
Agreed in Design WG that additional limit of 8 characters can be added to the spec, with a unit test for all versions ≥ 48.
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.
It says that each identifier is a BCP-47 subtag. It also says that the code is a single identifier.
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.
This was a limit of 8 characters on the code attribute.
|
That isn't what we agreed on.
…On Thu, Jan 15, 2026, 02:31 Robert Bastian ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/ldml/tr35-dates.md
<#5283 (comment)>:
> @@ -1098,8 +1098,8 @@ For example, the following means that the two eras from calendar "gregorian" sho
</calendar>
```
-Each `era` element has a `code` attribute and optional `aliases` attributes that define invariant strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below).
-The `code` is unique within the calendar, and the `aliases` are space-separated identifiers, each also unique within the calendar.
+Each `era` element has a `code` attribute and optional `aliases` attributes that define stable strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below).
+The `code` defines the primary identifier for the era, and `aliases` are space-separated additional identifiers. All identifiers are valid BCP-47 subtags, i.e. 2-8 ASCII alphanumeric characters.
It says that each identifier is a BCP-47 subtag. It also says that the
code is a single identifier.
—
Reply to this email directly, view it on GitHub
<#5283 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMAMQG72FGS35HAHCCT4G5UAPAVCNFSM6AAAAACRNOYWU6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNRUHE2DMMJQGI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Then we need to discuss this again because that's what I understood. |
| Each `era` element has a `code` attribute and optional `aliases` attributes that define invariant strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below). | ||
| The `code` is unique within the calendar, and the `aliases` are space-separated identifiers, each also unique within the calendar. | ||
| Each `era` element has a `code` attribute and optional `aliases` attributes that define stable strings for identifying the eras. These are more mnemonic than the `type` identifiers (see below). | ||
| The `code` defines the primary identifier for the era, and `aliases` are space-separated additional identifiers. All identifiers are valid BCP-47 subtags, i.e. 2-8 ASCII alphanumeric characters. |
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.
This was a limit of 8 characters on the code attribute.
|
That is, it would be possible to have |
CLDR-19157
ALLOW_MANY_COMMITS=true