CLDR-17844 Modify the date report #3920

macchiati · 2024-07-31T03:59:45Z

Modify the date report to help figure out what is happening with the date formats, and present the user with more and clearer information.

Added BidiUtils with utilities for display of examples/reports
Small changes to CodePointEscaper, plus support for printing a table of escape IDs in a report.
DateTypeFormats
- General cleanup
- Add a paragraph of text underneath Patterns to explain the format. Clarify that this is for the default number system, and explain more about the format
- In an RTL language there may be two examples, the top one is the dir=, and second is dir=auto with a distinct background. This allows vetters to see the contrast, and correct if important.
- Add a line showing the escaped text
- Add a warning when the reordering causes numbers or words to collide (all items currently pass)
- If there are any escaped characters in any cell, print a Key at the end for those characters.
- In the options for running manually, if there is a filter, then it is used; otherwise the locales are filtered to just the availableLanguages (for the charts).
This PR completes the ticket.

ALLOW_MANY_COMMITS=true

macchiati · 2024-07-31T04:25:57Z

Once the charts resync, the following should show up:
https://unicode.org/cldr/charts/46/verify/dates/he.html
https://unicode.org/cldr/charts/46/verify/dates/ar.html
https://unicode.org/cldr/charts/46/verify/dates/ar_DZ.html
https://unicode.org/cldr/charts/46/verify/dates/ar_EG.html

Here are snapshot of changes:

macchiati · 2024-07-31T13:28:23Z

I changed to rtl vs auto, which resolved a bunch of problems. Also added for comparison:

https://unicode.org/cldr/charts/46/verify/dates/fa.html
https://unicode.org/cldr/charts/46/verify/dates/de.html

Will update with screenshots.

macchiati · 2024-07-31T15:25:37Z

Arabic

The one locale (out of those I checked) that shows an alert is Farsi. But I have to check that over because it doesn't seem like the alert test is quite right yet.

Farsi

pedberg-icu

Nice!

AEApple

Thank you!

macchiati · 2024-08-01T13:46:24Z

Final cleanup for this PR. Am getting now no "alerts" indicating letter or number collisions. Also regenerated the charts.

macchiati · 2024-08-01T16:08:33Z

Just updated the description, so would appreciate review.

btangmu

looks great except minor nitpicking comments

btangmu · 2024-08-01T19:57:46Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/BidiUtils.java

+    }
+
+    /**
+     * Return a list of the , where each span is a sequence of:


word missing before the comma? Actually this whole comment looks unattached to anything, obsoleted by the following comment Gets the fields

btangmu · 2024-08-01T20:01:50Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/BidiUtils.java

+     * @return a set of fields, in the same order as found in the text but duplicates removed (ike
+     *     LinkedHashSeet).
+     */
+    public static Set<String> getFields(String reordred, Set<String> result) {


reordred should be reordered and the comment above should have @param reordered and @param result

btangmu · 2024-08-01T20:06:34Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/BidiUtils.java

+            return "";
+        }
+        // there can still be differences within a field of OTHERS, that we  ignore.
+        // EG ⚠️ 20,28,2B; 2B,28,20 " (+" vs " (+"


2B,28,20 should be " ( "?

btangmu · 2024-08-01T20:09:49Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/BidiUtils.java

+                .collect(Collectors.joining(LRM + ", " + LRM, LRM, LRM));
+    }
+
+    public static String alphagram(String string) {


alphagram seems unused?

btangmu · 2024-08-01T20:17:13Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/CodePointEscaper.java

+            UnicodeSet escapesToShow, String tableOptions, String cellOptions) {
+        if (!escapesToShow.strings().isEmpty()) {
+            throw new IllegalArgumentException("No strings allowed in the unicode set.");
+        }


maybe also confirm tableOptions and cellOptions start with spaces if not empty -- either throw IllegalArgumentException or insert the spaces if missing

btangmu · 2024-08-01T20:18:11Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/CodePointEscaper.java

+                final String id = cpe.name();
+                final String shortName = cpe.getShortName();
+                final String description = cpe.getDescription();
+                addREsult(result, tdPlus, id, shortName, description);


is capital E in addREsult a typo?

btangmu · 2024-08-01T20:21:45Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/DateTimeFormats.java

+                                    : "However, two examples are provided if the locale is right-to-left, like Arabic or Hebrew, "
+                                            + "<i>and</i> the paragraph direction can cause a different display. "
+                                            + "The first has a <b>RTL</b> paragraph direction, "
+                                            + "while the second has a <b>auto</b> paragraph direction (LTR unless the first 'strong' character is RTL) "


a --> an (before auto)

btangmu · 2024-08-01T20:23:20Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/DateTimeFormats.java

+                            + spanEnd
+                            + " shows those characters with short IDs ❰…❱: see the <b>Key</b> below the table. "
+                            + "So that the ordering of the characters in memory is clear, they are presented left-to-right one at a time. "
+                            + "so that the placement is clear. "


"so that the placement is clear. " looks like leftover variant, delete?

macchiati · 2024-08-02T00:47:08Z

Thanks!
I went ahead and merged this, since I'm going to do a followup PR anyway. (Maybe more than one.) So I'll add this to the TODOs in the ticket.

CLDR-17844 Modify the date report

d9b2073

macchiati requested review from pedberg-icu and srl295 July 31, 2024 03:59

github-actions bot assigned macchiati Jul 31, 2024

macchiati requested a review from AEApple July 31, 2024 04:00

macchiati marked this pull request as draft July 31, 2024 04:22

CLDR-17844 Change to auto vs RTL comparison

2d8ad0b

pedberg-icu previously approved these changes Jul 31, 2024

View reviewed changes

AEApple previously approved these changes Jul 31, 2024

View reviewed changes

srl295 previously approved these changes Jul 31, 2024

View reviewed changes

CLDR-17844 cleanup

595dde3

macchiati dismissed stale reviews from srl295, AEApple, and pedberg-icu via 595dde3 August 1, 2024 13:43

macchiati marked this pull request as ready for review August 1, 2024 13:45

macchiati requested review from srl295, pedberg-icu and AEApple August 1, 2024 13:45

macchiati requested review from btangmu and DraganBesevic August 1, 2024 19:51

btangmu approved these changes Aug 1, 2024

View reviewed changes

macchiati merged commit e302d06 into unicode-org:main Aug 2, 2024
12 checks passed

macchiati deleted the CLDR-17844-Modify-the-date-report branch August 2, 2024 00:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDR-17844 Modify the date report #3920

CLDR-17844 Modify the date report #3920

macchiati commented Jul 31, 2024 •

edited

Loading

macchiati commented Jul 31, 2024

macchiati commented Jul 31, 2024

macchiati commented Jul 31, 2024 •

edited

Loading

pedberg-icu left a comment

AEApple left a comment

macchiati commented Aug 1, 2024

macchiati commented Aug 1, 2024

btangmu left a comment

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

btangmu Aug 1, 2024

macchiati commented Aug 2, 2024

CLDR-17844 Modify the date report #3920

CLDR-17844 Modify the date report #3920

Conversation

macchiati commented Jul 31, 2024 • edited Loading

macchiati commented Jul 31, 2024

macchiati commented Jul 31, 2024

macchiati commented Jul 31, 2024 • edited Loading

pedberg-icu left a comment

Choose a reason for hiding this comment

AEApple left a comment

Choose a reason for hiding this comment

macchiati commented Aug 1, 2024

macchiati commented Aug 1, 2024

btangmu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macchiati commented Aug 2, 2024

macchiati commented Jul 31, 2024 •

edited

Loading

macchiati commented Jul 31, 2024 •

edited

Loading