Provide support for translating plural forms

[ma2ciek]

Suggested merge commit message (convention)

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.

---

Additional information

The translate function in the translation-service was marked as protected and renamed to _translate. All features depending on the translation mechanism should use the Locale#t() function instead.

The new format of the window.CKEDITOR_TRANSLATIONS:

{
   pl: {
      dictionary: {
         'Cancel': 'Anuluj',
         'Add space': [ 'Dodaj spację', 'Dodaj %0 spacje', 'Dodaj %0 spacji' ]
      }, 
      // A function that returns the plural form index.
      // Actually it's more like n => n == 1 ? 0 : n % 10 >= 2 && n % 10 <= 4 && ( n % 100 < 10 || n % 100 >= 20 ) ? 1 : 2
      getPluralForm: n => n !==1
   }
   // other languages...
}

[coveralls]

Coverage remained the same at 100.0% when pulling 3d8c423 on i/6526 into fb13d9d on master.

[scofalik]

In general the code is fine but I am not happy with docs.

Especially two things:

1. There are no new docs for t().
2. There is no word about context.

Also, please change String[] to Array.<String> in docs.

When you will write new docs, please remember about an example that has plural forms and multiple values to interpolate and mention that the first value will be used for choosing a plural form.

I also don't like docs for translate() -- it is not your fault, somebody wrote them in a messy way earlier. I would reword them but honestly, I am not sure where this method is used, so before we waste time for those docs, let's think if there will be any gain in improving them.

[scofalik]

Some changes in docs.

[ma2ciek]

I used the amount word across these two files. But maybe the quantity fits better?

cc @scofalik

[mlewand]

Interesting to see that you decided to handle such a tricky case as multiple plural version - what is use case for it? 🙂

ckeditor5-utils/src/translation-service.js

Lines 77 to 79 in 33acb99

	if ( !window.CKEDITOR_TRANSLATIONS[ language ] ) {
	window.CKEDITOR_TRANSLATIONS[ language ] = {};
	}

- this change introduces a breaking change to public API that should be noted.

• why did you go with the string + context prop combo rather than proposed id property in the end?
  • I'm a little tentative about using context + string to build a translation id, as context seems to be prone to changes during product lifetime (like, stupid "a", "an", "the" changes). Instead I think that id will be a better option, and since we'll have to inline all contextes, why not inlining all the ids in a single run?

This is a major change and getting it from the API docs alone is rather difficult task. We should add a guide/doc on i11n matter, which would include feature added with this PR. It's a perfect moment to add this guide in this release (ofc should be a followup task).

[ma2ciek]

 Interesting to see that you decided to handle such a tricky case as multiple plural version - what is use case for it? 🙂

There's a lot of use cases in our track changes code where the add spaces (3) doesn't look as good as we want.

 - this change introduces a breaking change to public API that should be noted.

You mean the whole format of the window.CKEDITOR_TRANSLATIONS? Certainly it needs to be mentioned.

why did you go with the string + context prop combo rather than proposed id property in the end?

• I'm a little tentative about using context + string to build a translation id, as context seems to be prone to changes during product lifetime (like, stupid "a", "an", "the" changes). Instead I think that id will be a better option, and since we'll have to inline all contextes, why not inlining all the ids in a single run?

The context + string combo will be an id only in situations where the context will be a part of the t( messageObject ) form. Therefore it will be a little bit shorter than our long contexts and hopefully it won't be changed so often. But that's for sure a thing to remember.

And why it's no id? After some f2f talks with @scofalik we decided to change it as the context + string should be unique enough for creating a message id, and this doesn't require writing the same twice (as the id and the context would probably quite similar). But there're some obvious cons of it as it's less verbose, creates context + message, that's hard to change in the future and maybe not intuitive. As I think now that changing the message string or message context will break translations I'm leaning towards the id 😂 

This is a major change and getting it from the API docs alone is rather difficult task. We should add a guide/doc on i11n matter, which would include feature added with this PR. It's a perfect moment to add this guide in this release (ofc should be a followup task).

I was thinking about it too. There're a few things worth noting. We have an internal guide (which is going to be outdated after this task). There's a guide for setting the UI language - https://ckeditor.com/docs/ckeditor5/latest/features/ui-language.html, but we lack a guide for a 3rd-party plugin creator.

[scofalik]

As far as id is concerned... Isn't it a problem right now as well? You introduce some translation as t( 'Foobar' ) and then it becomes a problem if we would like to change it. Isn't it the same here? How it was solved until now?

What I don't like about id is that it is repeating the same information again :S.

EDIT: BTW. we are not going to inline all contextes. We will still keep some of them in context.json.

[ma2ciek]

As far as id is concerned... Isn't it a problem right now as well? You introduce some translation as t( 'Foobar' ) and then it becomes a problem if we would like to change it. Isn't it the same here? How it was solved until now?

Yes, but only for message strings as message contexts (longer and therefore changing more often) are kept in contexts.json file. Note that in our cases context and string are long strings  and are changing 3 times a day currently 😄 

Actually having the messageId in track changes would allow us to change the message string or message context without any trouble in the future. With the context + string where any one can't change the situation is quite opposite. When a string would be changes without changing id Transifex would also remove all translations but with better effect - https://docs.transifex.com/projects/updating-content - it would preserve the history of string translation and so on.

[ma2ciek]

That's why I chose this new interface for the t() function - you can still extend the message with the id property in the future 😄

[scofalik]

If you think it is correct, go with it.

[mlewand]

There's a lot of use cases in our track changes code where the add spaces (3) doesn't look as good as we want.

👍 

You mean the whole format of the window.CKEDITOR_TRANSLATIONS? Certainly it needs to be mentioned.

That's right, change of structure in this global variable is a BC. So needs to be reflected in changelog.

This is a major change and getting it from the API docs alone is rather difficult task. We should add a guide/doc on i11n matter

Guide task extracted to ckeditor/ckeditor5#6646 - this is something we can get back to later on.

[mlewand]

Couple of last suggestions, LGTM.