Skip to content
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

CLDR-17842 Add semantic skeleton spec text #4031

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CLDR-17842 Add semantic skeleton spec text #4031

wants to merge 1 commit into from

Conversation

sffc
Copy link
Member

@sffc sffc commented Sep 10, 2024

CLDR-17842

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

@sffc sffc changed the title Add semantic skeleton spec text CLDR-17842 Add semantic skeleton spec text Sep 10, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@sffc sffc marked this pull request as ready for review September 10, 2024 02:09
@sffc
Copy link
Member Author

sffc commented Sep 10, 2024

Please advise on how and where I should indicate that this is a technology preview.

@macchiati
Copy link
Member

macchiati commented Sep 10, 2024 via email

docs/ldml/tr35-dates.md Outdated Show resolved Hide resolved
| { Hour, Minute } | 4:03 pm | 16:03 |
| { Hour, Minute, Second } | 4:03:51 pm | 16:03:51 |

Note: Minute and Second are not valid time field sets on their own because they do not refer to a particular time of day. They must be interpreted in the context of an explicit hour.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note somewhere that formatting durations, like 3 minutes 12 seconds (or 3:12) is not handled through the Skeleton mechanisms.

docs/ldml/tr35-dates.md Outdated Show resolved Hide resolved

The _year style_ defines the level of precision to use when displaying the year. There are three choices:

1. **Auto:** Display the year with full or partial precision, and display the era if needed to disambiguate the year, depending on locale, calendar, and length. Example: Jan. 1, '00
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ‘00 instead of '00; add another example esp for Auto. We need to make sure that people are not mislead into thinking that Auto is confined.

Suggested change
1. **Auto:** Display the year with full or partial precision, and display the era if needed to disambiguate the year, depending on locale, calendar, and length. Example: Jan. 1, '00
1. **Auto:** Display the year with full or partial precision, and display the era if needed to disambiguate the year, depending on locale, calendar, and length. Example: Jan. 1, ‘00 or 1 January 2000

Now that I think about it, it might be clearer with a little table for each of the style options, instead of a numbered list.

Copy link
Member Author

@sffc sffc Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used a lot of tables, but I didn't here because there was too much inline text, which doesn't render very well with tables.

EDIT: In this specific case, I added a table.


The [year style](#Semantic_Skeleton_Year_Style) should change the skeleton for all lengths as follows:

- Auto: No change from datetimeSkeleton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note: the datetimeSkeleton may already be y; and may include G already, subject to locale preferences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar notes elsewhere, just for clarity.

@sffc sffc requested a review from macchiati September 11, 2024 04:01
@sffc
Copy link
Member Author

sffc commented Sep 11, 2024

I believe I addressed all the feedback.

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@sffc
Copy link
Member Author

sffc commented Sep 16, 2024

Squashed

@sffc
Copy link
Member Author

sffc commented Sep 17, 2024

I'm not sure what the test failure is for:

Error: (TestAll.java:165) Error: (TestAll.java:165) java.lang.IllegalArgumentException: Unknown system zone id: Etc/Unknown

@macchiati
Copy link
Member

I think someone mentioned that that is an ICU problem (that I think was fixed, and that we just need to upgrade to the latest ICU). @DraganBesevic @pedberg-icu

@sffc sffc closed this Sep 17, 2024
@sffc sffc reopened this Sep 17, 2024
@sffc
Copy link
Member Author

sffc commented Sep 17, 2024

@eggrobin PTAL

@macchiati
Copy link
Member

The build problem looks completely unrelated to this PR. @srl295 any ideas?

Check failure on line 165 in tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestAll.java

GitHub Actions
/ build
 (TestAll.java:165)  Error: (TestAll.java:165) java.lang.IllegalArgumentException: Unknown system zone id: Etc/Unknown

@macchiati
Copy link
Member

Shane, Steven says you need to remerge or rebase on top of latest main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants