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 translating plural forms (i18n vol 3) #6526

Closed
ma2ciek opened this issue Apr 1, 2020 · 2 comments · Fixed by ckeditor/ckeditor5-dev#614
Closed

Support translating plural forms (i18n vol 3) #6526

ma2ciek opened this issue Apr 1, 2020 · 2 comments · Fixed by ckeditor/ckeditor5-dev#614
Assignees
Labels
domain:i18n The issue reports a problem with internalization / translation mechanisms type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 1, 2020

📝 Provide a description of the new feature

Reasoning

The current implementation of the internalization process doesn't support translating plural forms. Supporting many plural forms is necessary in cases where there's an unknown number of elements like Add %0 bananas and there is an unknown language and not known number and rules of its plural forms. For sure we could write the Add bananas (%0) but this looks much less friendly and IMO it's a little bit distractive.

Required changes an checks

  • Check if the plural forms work on the Transifex. If they can be easily downloaded and uploaded. It could be done on a Transifex new demo project.
  • Ensure that the upload/download scripts work well for plural forms. Fix them if necessary.
  • A function or functions that could handle plural forms or more advanced translation cases for single forms are needed. (ckeditor5-utils)
    • All functions should work if the translation files are missing and should default to the English ones.
    • A function that can return either translation fragments or an interpolated string would be useful in some scenarios, however, this would conflict with the current API.
    • All necessary t() function changes should be backward-compatible.
  • A tool that collects translation strings from the t() function needs to support new function(s) too. (ckeditor5-dev)
  • Translation files should provide all information needed to the translate() function in ckeditor5-utils. (ckeditor5-dev)
    • a single and all plural forms (an array?) for the given string
    • we could use window.Intl.PluralRules but it is not implemented in Safari, so each translation file should include its own plural rules

Possible further improvements

  • upgrade tooling to e.g. gettext-extract , xgettext etc.
  • move contexts directly to t() function invocations, remove contexts.json file (?) This would be only valuable if we'd need to have contexts directly in new functions
  • make easy adding 3rd-party translations for plugin developers, create a guide for it → issue

Limitations

API proposal

Translation functions

Proposal 1: Full sentences

Without the function returning intermediate results we could just add an overload to the known t() function:

t( singleForm, pluralForm?, [ ...values ]? ) 
  • and do it in a backward-compatible way. This way the plural form could be filled based on the amount of the first value and the rest of placeholders could be filled as well.

The above API could be used in the following way:

t( 'Add a banana', 'Add %0 bananas', [ bananas ] );
t( 'Remove a %1 banana', 'Remove %0 %1 bananas', [ bananas, 'sweet' ] );

Note that concatenating strings could be quite dangerous due to the fact that languages can have various declension rules so we can't do the following:

t( 'Add' ) + ' ' + t( 'a banana', '%0 bananas', [ bananas ] );

The styling should be added directly into the code like as the t() function produces strings and there's no way to determine later which part should be be bolded. The bolded text could be done only this way:

t( '*Add* a *banana*', '*Add* %0 *bananas*', [ bananas ] );

So after the translation phases these styling could be e.g. wrapped in bold.

Proposal 2 - Add additional support for defining translation ids

Add contexts/translation ids to translation strings so some translations can be reused without worrying about different declension rules.

E.g.:

t( 'Add' ) + ' ' + ct( 'Add %0 bananas', 'a banana', '%0 bananas', [ bananas ] );

Pluses

  • less number of translations (especially for languages with many declensions.

Minuses

  • one or two translation functions more to handle in each translation step
  • inconsistency between ct and t functions - in the ct function we'd need to use contexts as translation id or add a unique id as the first argument
  • more work

Proposal 3 - return an object instead of a string for more customizable usages

Create a function that returns an object with translation fragments

e.g.:

const translation = ct( 'Add %0 bananas', 'a banana', '%0 bananas', [ bananas ] );
    
translation.toString() // -> 'Add 4 bananas'
translation.toParts(); // ->
// [ { type: 'literal', value: 'Add ' },
// { type: 'value', value: 4 },
// { type: 'literal', value: ' bananas' } ]

Pluses

  • more customizable translations, no need for asterisks in the code to generate bold afterward

Minuses

  • an API much different than the t() function API while the function name seems to be familiar
  • 2 more dedicated functions (one for single forms and one for plural forms).
  • complicated code

With the toString() function defined, we could also use it indirectly in the string concatenations as we use the t() function currently. However such a not-explicit way seems a little bit scary.

Translation files

The translation files need to have defined translation rules like:

"Plural-Forms: nplurals=3; plural=(n==1 ? 0 : n%10>=2 && n%10<=4 && (n%100<12 || n%100>14) ? 1 : 2);\\n"

They also need to include these plural forms:

{
    '*Add* a *banana*': [ 'Dodaj banana', 'Dodaj %0 banana', 'Dodaj %0 banany', 'Dodaj %0 bananów' ]
}

Refs

cc @Reinmar @scofalik @mlewand


If you'd like to see this feature implemented, add a 👍 reaction to this post.

@ma2ciek ma2ciek added the type:feature This issue reports a feature request (an idea for a new functionality or a missing option). label Apr 1, 2020
@ma2ciek ma2ciek self-assigned this Apr 1, 2020
@ma2ciek ma2ciek added this to the iteration 31 milestone Apr 1, 2020
@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 3, 2020

Extending the 2. proposal we could add the support for the t( translationOptions, values ) instead of adding more params to the t() function, with the following interface for translationOptions:

interface TranslationOptions {
   // A string id that would be used for translation uniqueness.
   // The value defaults to the `string`'s value if it isn't set.
   // We could also use it as a translation context to not define two similar things.
   // This way that string could be also used by translators.
   // E.g. `'Show/Hide %0 editors'`.
   id?: string;

   // A translation string. Required. E.g. `'editor'`.
   string: string;

   // A plural form of the translation. E.g. `'%0 editors'`.
   // It can be easily ommited if the translation will not have plural forms.
   plural?: string
}

This way we could still use the t() function but provide 2 overloads.

t( translationString, [ ...values ] ) would be an equivalent to t( { string: translationString, id: translationString }, [ ...values ] ) but the second API would provide options for more advanced usages.

One more advantage is that this API is open-ended for future enhancements. In contrast, the previously proposed APIs would be impossible to change in the future without introducing breaking changes.

I've checked and both, parsing (when looking for translation strings and other things) and obfuscation should work for this API.

@ma2ciek ma2ciek added the domain:ui/ux This issue reports a problem related to UI or UX. label Apr 3, 2020
@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 8, 2020

As an update, I'll write that I chose the above API as it seems the most flexible, extendible and descriptive and uses the old t() function that everyone already knows.

@ma2ciek ma2ciek added domain:i18n The issue reports a problem with internalization / translation mechanisms and removed domain:ui/ux This issue reports a problem related to UI or UX. labels Apr 10, 2020
mlewand added a commit to ckeditor/ckeditor5-utils that referenced this issue Apr 23, 2020
Feature: Provided support for plural forms internalization. Part of ckeditor/ckeditor5#6526.

MINOR BREAKING CHANGE: The `translate` function from the `translation-service` was marked as protected. See #334.
MINOR BREAKING CHANGE: The format of translations added to the editor has been changed. If you use `window.CKEDITOR_TRANSLATIONS` please see #334.
mlewand added a commit that referenced this issue Apr 23, 2020
Internal: Aligned snippet adapter to changes in CKEditorWebpackPlugin. Part of #6526.
mlewand added a commit to ckeditor/ckeditor5-dev that referenced this issue Apr 23, 2020
Feature: Provided support for plural forms i18n. Closes ckeditor/ckeditor5#6526. Closes ckeditor/ckeditor5#988.

MINOR BREAKING CHANGE: Omitting the language property in the CKEditorWebpackPlugin will not have any effect from now. This means that in both cases only the main language (language) will be added to the main bundle and translations for other languages will be saved in separate files.
MAJOR BREAKING CHANGE: The translation process no longer creates short ids for message strings. From now, the source code will not be changed by the translation process, translations for the main language will be added to the bundle(s) and translations for other languages will be outputted to separate executable JS files.
mlewand added a commit that referenced this issue May 1, 2020
Feature: Provided support for plural forms internalization. Part of #6526.

MINOR BREAKING CHANGE: The `translate` function from the `translation-service` was marked as protected. See #334.
MINOR BREAKING CHANGE: The format of translations added to the editor has been changed. If you use `window.CKEDITOR_TRANSLATIONS` please see #334.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:i18n The issue reports a problem with internalization / translation mechanisms type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant