-
Notifications
You must be signed in to change notification settings - Fork 388
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-17014 No code fallbacks for language/script paths #4290
Conversation
-Use the extra-paths mechanism instead of code-fallback; new ExtraPaths.java -For each type (language/script), create the set of extra paths only once -Change es-419 to es (419) in localeDisplayName.txt so TestLocaleDisplay passes -Revise testExtraPaths, testGetPaths, so they pass -Comments
This PR is very similar to #4254, but in addition to language paths it also applies to script paths, and the new class ExtraPaths is refactored to facilitate adding more types in addition to language/script |
@macchiati please let me know when it's OK to deploy this to cldr-staging for testing screenshot running locally: http://localhost:8888/cldr-apps/v#/aa/Scripts/44d4d43894cfbd45 compare production https://st.unicode.org/cldr-apps/v#/root/Scripts/44d4d43894cfbd45 |
Ok to deploy now (or when you are back at work); and everyone should try it
out, since there will be differences in UI. Best to try with a locale at
(say) Basic but set the Coverage up to Moderate to see how it behaves.
We should also make sure to include a notice about this change in the
release notes. (Maybe we should have a label releaseNote for that in
tickets? @***@***.***)
…On Fri, Jan 17, 2025 at 12:27 PM Tom Bishop ***@***.***> wrote:
@macchiati <https://github.com/macchiati> please let me know when it's OK
to deploy this to cldr-staging for testing
screenshot running locally:
http://localhost:8888/cldr-apps/v#/aa/Scripts/44d4d43894cfbd45
image.png (view on web)
<https://github.com/user-attachments/assets/aee1dfd6-5b47-4c3b-9e5d-eb596529a0e5>
compare production
https://st.unicode.org/cldr-apps/v#/root/Scripts/44d4d43894cfbd45
image.png (view on web)
<https://github.com/user-attachments/assets/a774b24a-4e57-43e0-b304-c6cec9bba6be>
—
Reply to this email directly, view it on GitHub
<#4290 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMFAWNQJN7NYI7RKGQL2LFRSZAVCNFSM6AAAAABVMTF66SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJZGEZTMOBRG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Per today's infra meeting, this is now ready for review |
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.
LGTM. Need to get those lists out of code though (which isn't this PR)
private void adjustCodeSet(Set<String> codes) { | ||
if (nameType == NameType.LANGUAGE) { | ||
codes.remove(LocaleNames.ROOT); | ||
codes.addAll( | ||
List.of( | ||
"ar_001", "de_AT", "de_CH", "en_AU", "en_CA", "en_GB", "en_US", | ||
"es_419", "es_ES", "es_MX", "fa_AF", "fr_CA", "fr_CH", "frc", "hi_Latn", | ||
"lou", "nds_NL", "nl_BE", "pt_BR", "pt_PT", "ro_MD", "sw_CD", "zh_Hans", | ||
"zh_Hant")); | ||
} | ||
} | ||
|
||
private void addAltPaths() { | ||
switch (nameType) { | ||
case LANGUAGE: | ||
addAltPath("en_GB", "short"); | ||
addAltPath("en_US", "short"); | ||
addAltPath("az", "short"); | ||
addAltPath("ckb", "menu"); | ||
addAltPath("ckb", "variant"); | ||
addAltPath("hi_Latn", "variant"); | ||
addAltPath("yue", "menu"); | ||
addAltPath("zh", "menu"); | ||
addAltPath("zh_Hans", "long"); | ||
addAltPath("zh_Hant", "long"); | ||
break; | ||
case SCRIPT: | ||
addAltPath("Hans", "stand-alone"); | ||
addAltPath("Hant", "stand-alone"); | ||
} | ||
} |
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.
why are these two tables in code instead of in data?
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.
Could be, but best to be in a different ticket.
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.
Moving to a data file is an interesting idea. This ticket moves the code from XMLSource.java to ExtraPaths.java, and so far only for language/script paths. There's still a lot of code for other kinds of extra paths in CLDRFile.java, and for other kinds of "constructed" paths (like currencies) in XMLSource.java. A plan to move such code to a data file should probably take into account not only the code here but also what remains in CLDRFile and XMLSource. Not all of those paths in CLDRFile currently have corresponding NameType values; dayPeriods paths for example don't. All those in XMLSource do have corresponding NameType values. Even if we keep the fallback values for currencies, etc., we might consider taking them out of XMLSource.java and treating them more like extra paths. There could be more of a comprehensive framework rather than scattered code/data.
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 seems like perhaps it's a coverage level issue or something. Not sure.
-Use the extra-paths mechanism instead of code-fallback; new ExtraPaths.java
-For each type (language/script), create the set of extra paths only once
-Change es-419 to es (419) in localeDisplayName.txt so TestLocaleDisplay passes
-Revise testExtraPaths, testGetPaths, so they pass
-Comments
CLDR-17014
ALLOW_MANY_COMMITS=true