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

Support plural string resource #4519

Merged
merged 16 commits into from Mar 25, 2024
Merged

Conversation

paxbun
Copy link
Contributor

@paxbun paxbun commented Mar 20, 2024

Ports a part of Unicode's ICU in pure Kotlin and implements Android-style plural string resource support. Fixes #425.

Changes

  • Added org.jetbrains.compose.resources.intl.{PluralCategory, PluralRule, PluralRuleList}, which parses and evaluates scripts in Unicode's Locale Data Markup Langauge.
  • Copied plurals.xml from Unicode's CLDR.
  • Added GeneratePluralRuleListsTask, which parses plurals.xml and generates required Kotlin source codes.
  • Added PluralStringResource, pluralStringResource, or getPluralString, corresponding to StringResource, stringResource, or getString.
  • Modified ResourcesSpec.kt so the generated Res class exposes Res.plurals.

Potential Further Improvements

  • Allow configuring the default language in the compose.resources {} block ([gradle] Add DSL to configure compose resources #4482) to determine the default pluralization rule (or just presume English as default)
  • Move the parser logic to the Gradle plugin and generate pluralization rules in Res only for languages used in composeResources

@terrakok
Copy link
Collaborator

@paxbun Thank you for the great contribution! I made a backward PR to your branch with some refactorings. And I would like to ask to change a logic a bit: not to crash an app when user hasn't provided any pluralization:

kotlin.IllegalStateException: String ID=`new_message` does not have the pluralization MANY for quantity 0!

I guess it's better to use a some default and print a warning to the log. what do you think?

@paxbun
Copy link
Contributor Author

paxbun commented Mar 23, 2024

@terrakok Thanks for the comment! I merged your changes and renamed the properties of GeneratePluralRuleListsTask (cause they're now files, not directories).

I guess it's better to use a some default and print a warning to the log. what do you think?

The current implementation of Android SDK also throws an Exception when there is no proper pluralization, although mine did not fallback to other when the right one was not found (I've just added the fallback logic).

We can return an empty string, but in that case we also have to care about ones with format arguments because String.replaceWithArgs will throw an exception for an empty string.

@terrakok
Copy link
Collaborator

The reason of the red tests is the compose gradle plugin uses published version, which doesn't have the PluralResource yet.

We have to divide the PR on two:

  1. the first part is library -> to publish it as dev version
  2. the gradle plugin part -> to depend on dev version of library

@terrakok terrakok merged commit 2b1bf65 into JetBrains:master Mar 25, 2024
3 of 12 checks passed
@terrakok
Copy link
Collaborator

I will do it manually

@terrakok
Copy link
Collaborator

BTW, you forgot to handleSpecialCharacters

val pluralCategory = PluralCategory.fromString(
    element.getAttribute("quantity"),
) ?: return@mapNotNull null
val rawString = element.textContent.orEmpty()
pluralCategory to handleSpecialCharacters(rawString) // <--- here

could you fix it?

@paxbun
Copy link
Contributor Author

paxbun commented Mar 26, 2024

Of course! Here is the PR as you requested: #4543

terrakok pushed a commit that referenced this pull request Mar 26, 2024
A follow-up PR for #4519, which handles special characters for quantity
strings.
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.

String resources for internationalization (i18n)
2 participants