[Resources] Support BCP 47 script code qualifiers in resource folders#5582
[Resources] Support BCP 47 script code qualifiers in resource folders#55822KAbhishek wants to merge 14 commits into
Conversation
|
@terrakok thank you for the review comments, I will be resolving those and get back |
|
In one comment you write you also added support for UN M49 region codes.
(see #5582 (comment)) |
|
As a side-note, I always wondered about the odd Reading the Android documentation on this matter, it looks like the above mentioned syntax simply came to be for historic reasons. Only with Android 7.0, the So, wouldn't it be fine to just use 1:1 IETF BCP47 language tags as directory names instead of inheriting this legacy Android syntax? E.g.
instead of
Of course, using Disclaimer: I am no maintainer here, this is just my opinion. It remains with maintainers such as @terrakok to make any decisions in this regard. I just want to allude to that now is probably the only sensible opportunity to drop that legacy syntax. |
787cf07 to
8ef360f
Compare
@westnordost can you please help by creating an issue for this on YouTrack? I think it will be better to take the UN M49 mapping as a subsequent fix and not include it on this PR - I can work on adding it |
@westnordost Appreciate the suggestion, but for this PR I'd like to keep scope tight: support what Android's resource system already supports - b+ and lang-rREGION, so resources can be ported from Android to CMP without extra steps. Moving to IETF-pure folder syntax (sr-Latn-RS, en-GB) is a worthwhile discussion but I think it's a separate concern from script-code support and would need its own deprecation story for the existing region syntax. Happy to revisit as a follow-up issue if needed. |
|
@terrakok I have resolved the suggestions and did some clean up, can you please take another look 🙏🏽 |
Right, but the resource directory names for Compose Multiplatform Resources already deviate from those from Android: Android uses |
|
One note about UN49 support: I wrote earlier...
This is wrong, or at least not helpful. While in e.g. JavaScript, |
f31d8c9 to
bb1e878
Compare
|
@terrakok just wanted to follow up for a re-review, can you please take another look |
Compose Multiplatform resource parsing rejected valid Android BCP folder
names like values-b+sr+Latn (script code) and values-b+es+419 (numeric
region) with "unknown qualifier" errors. The Gradle plugin split folder
names on '-' without understanding the b+lang+code segment format, and
qualifier validation only accepted 2-letter region codes.
Gradle plugin:
- Add parseAndroidFolderName() to handle BCPF b+lang+code segments
- Expand qualifier regexes for 4-letter ISO 15924 script codes and
3-digit numeric region codes
- Fix region code extraction (takeLast(2) -> removePrefix("r"))
- Emit ScriptQualifier in generated accessors with path validation
Runtime library:
- Add ScriptQualifier class
- Thread script through ResourceEnvironment on all platforms
- Extend filterByLocale: language -> script -> region, with fallback
to default when no match; empty-script environments (Compose Locale
does not expose script yet) prefer non-script items first
Tests:
- Add testGetPathByEnvironmentWithScript (script matching, empty-script
fallback, cross-language default)
- Add testBcpFolderQualifiers integration test for BCP folder parsing
Fixes https://youtrack.jetbrains.com/issue/CMP-4449/
Fixes JetBrains#4449
Split parseAndroidFolderName into parseComposeResourceLocaleQualifiers (orchestrator) and expandBcpQualifier (BCP segment expansion). The new structure supports folders that mix standard qualifiers with BCP segments (values-b+zh+Hant-dark) and multi-segment BCP chains (values-b+sr+Latn+RS). - Tighten path validation in addQualifiers with pathContainsBcpSubtag regex helper so multi-segment BCP paths are verified precisely - Malformed BCP segments now fall through to addQualifiers which reports "unknown qualifier", instead of being silently dropped - Expand integration tests: positive cases for script-only, numeric region, multi-segment, and mixed-qualifier BCP folders; negative test for malformed segments
Rewrite filterByLocale using val + early-return style consistent with
filterBy and filterByDensity in the same file. Extract filterByRegion
helper to avoid repeating region filter logic. Extract noLocaleItems
once to avoid the duplicated final-fallback filter.
The ScriptQualifier("") case from DefaultComposeEnvironment naturally
falls through the chain without needing a dedicated branch.
- Expand unit test with labeled assertions for each filter case:
language+script+region (exact and region-fallback), language
without script, language+region ignoring script, and no-language
default
DefaultComposeEnvironment.rememberEnvironment() hardcoded
ScriptQualifier(""), which made BCP 47 script-qualified folders (e.g.
values-b+zh+Hant) unreachable from stringResource() because
androidx.compose.ui.text.intl.Locale doesn't expose a script field.
Source the script from getSystemResourceEnvironment() instead, so the
Composable lookup path uses the same script the suspend
getString()/getDrawable()/etc. lookups already do.
addQualifiers verified "script after language" / "region after language" by re-scanning the resource folder path with a regex (pathContainsBcpSubtag), even though the parser had already produced the qualifiers in source order.
- Reject locale-shaped qualifiers (lang / Script / rRR / r000) that follow a b+... segment, e.g. values-b+sr+Latn-rRS or values-b+sr-rRS. Mixing the two locale syntaxes in one folder is ambiguous; the BCP segment must carry the full locale. - Enforce BCP 47 subtag ordering inside the b+... segment: language must come first, and subsequent subtags must follow language < script < region. Out-of-order combos like values-b+sr+RS+Latn now fall through to "unknown qualifier" instead of being silently accepted.
Two correctness fixes around the new ScriptQualifier support:
- filterByLocale: when the environment requests a specific script, never fall
back across scripts via region match. Script (e.g. Hans vs Hant, Latn vs Cyrl)
is a stronger signal than region; falling across scripts can show Traditional
Chinese to a user who explicitly asked for Simplified just because the region
matched. The cross-script byRegion fallback now runs only when the env script
is empty.
- DefaultComposeEnvironment: only borrow the system's script when the compose
locale language still matches the system's. If the app overrides Locale.current
(e.g. an in-app language picker) to a different language, ScriptQualifier("")
is used instead, so e.g. switching to "en" on a "ru-Cyrl" system no longer
produces a nonsensical "en-Cyrl" environment.
values-sr-Latn / values-sr-rRS-Latn were accepted as non-BCP script syntax. Reject them in parseComposeResourceQualifiers so the parser stays scoped to Android's b+... segment, and add matching invalid cases to testBcpFolderQualifiersInvalid. Also: - Add ScriptQualifier.isEmpty() helper and use it in filterByLocale. - Hoist BCP 47 / Android qualifier regexes to file-private vals shared by parseComposeResourceQualifiers, isLocaleShapedQualifier, and expandBcpQualifier. - Match the existing "Forbidden directory name '$dirName'! ..." error style for the two new validation messages.
filterBy already does exact-match-then-default-fallback for any Qualifier subtype, which is what filterByRegion was reimplementing specifically for RegionQualifier.
Bad BCP 47 folders previously laundered into the generic "unknown qualifier" message. Now expandBcpQualifier raises explicit errors for malformed subtags, missing leading language, and out-of-order subtags, and parseComposeResourceQualifiers rejects 'b+...' segments that aren't the first qualifier instead of letting them fall through.
Adds the missing positive case for a single-language b+ segment, plus invalid cases for empty subtags, uppercase-only language slot, and multi-region segments.
Two related cleanups around parseComposeResourceQualifiers: 1. Drop the early `if (parts.first().isEmpty()) return null`. Folders like '-foo' were silently skipped, leaving developers without diagnostics. 2. The six BCP-related errors had inconsistent location context: three pointed at `$dirName`, three at `$segment`, one had no suffix. Pass dirName through expandBcpQualifier so every error ends with "in '$dirName'." and names the offending qualifier or subtag when useful.
3efc792 to
b5110e2
Compare
|
@igordmn @eymar @Schahen @MatkovIvan Will be super helpful if you can take a look or let me know who can help with this 🙏🏽 |
Resource folder parsing rejected valid Android BCP 47 folder names like
values-b+sr+Latn(script code),values-b+zh+Hans+CN(multi-segment), andvalues-b+es+419(numeric region) with "unknown qualifier" errors when placed undercommonMain/composeResources.The Gradle plugin split folder names on
-without understanding theb+lang+codesegment format, and qualifier validation only accepted 2-letter region codes.This PR adds support for BCP 47 script codes (ISO 15924) and numeric region codes in resource folder qualifiers, propagates the script through both the suspend (
getString()) and Composable (stringResource()) lookup paths.Fixes CMP-4449
Fixes #4449
Testing
testGetPathByEnvironmentWithScriptinResourceTestcovers script matching (sr-Latn,sr-Cyrl,zh-Hans,zh-Hant), region narrowing, empty-script fallback, cross-language default resolution, and the script-priority guard.testDefaultComposeEnvironmentPropagatesSystemScriptandtestDefaultComposeEnvironmentDropsSystemScriptOnLanguageOverridecover the ComposablestringResource()script lookup path.testBcpFolderQualifiersandtestBcpFolderQualifiersInvalidintegration tests cover language-only, script, alpha-2 and numeric regions, multi-segment chains, and trailing theme/density qualifiers, plus rejection of malformed segments, out-of-order subtags, trailing locale qualifiers afterb+..., and standalone script qualifiers withoutb+.Release Notes
Fixes - Resources
values-b+sr+Latn,values-b+zh+Hans) and numeric region (values-b+es+419) qualifiers, including multi-segment locales and trailing theme/density qualifiers, incommonMainresource folders