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

Provide support for plural forms i18n, make more generic tools #614

Merged
merged 58 commits into from
Apr 23, 2020
Merged

Conversation

ma2ciek
Copy link
Contributor

@ma2ciek ma2ciek commented Apr 10, 2020

Suggested merge commit message (convention)

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.


Additional information

The t() function accepts now a string or an object with the following properties:

  • string (required)
  • id - used to identify ambiguous translations
  • plural - used when messages should support plural forms

This PR touches (but not resolves directly)

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

Checklist

  • CKEditor 5 manual tests work with new CKEditorWebpackPlugin
  • CKEditor 5 builds work with various options of new CKEditorWebpackPlugin
  • Integrations work with new CKEditorWebpackPlugin
  • Docs update for the whole translation process
  • Docs for the end-user
  • Generating CKE5 docs

@coveralls
Copy link

coveralls commented Apr 15, 2020

Coverage Status

Coverage decreased (-0.6%) to 91.052% when pulling 24a4209 on i/6526 into 1841693 on master.

@ma2ciek ma2ciek requested a review from mlewand April 20, 2020 15:17
@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 21, 2020

I've found an issue with generating the ~getPluralForm~ function from the Tx rules for some languages. Debugging... False alarm.

@mlewand
Copy link
Contributor

mlewand commented Apr 22, 2020

Merged the master branch.

Copy link
Contributor

@mlewand mlewand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TC:

  1. cd ckeditor5 # go to ckeditor 5 project
  2. yarn link @ckeditor/ckeditor5-dev-webpack-plugin # assuming that you already executed `yarn link` in ckeditor5-dev/packages/ckeditor5-dev-webpack-plugin
  3. yarn run manual -f table

I get following error:

[CKEditorWebpackPlugin] Error: Too many JS assets has been found during the compilation. You should add translation assets directly to the application from the `lang` directory or use the `allowMultipleJSOutputs` option to add translatiolanguage to all assets.
ns for the main language to all assets.

packages/ckeditor5-dev-webpack-plugin/README.md Outdated Show resolved Hide resolved
@mlewand
Copy link
Contributor

mlewand commented Apr 23, 2020

Building the docs also throws an error:

Building 127 snippets...
Building snippets for language "en": 40% (building)[CKEditorWebpackPlugin] Error: No JS asset has been found during the compilation. You should add translation assets directly to the application from the 'lang' directory.
Building snippets for language "en": 89% (module assets processing)[CKEditorWebpackPlugin] Error: Too many JS assets has been found during the compilation. You should add translation assets directly to the application from the `lang` directory or use the `allowMultipleJSOutputs` option to add translations for the main language to all assets.
Build completed in 60.019s

Building snippets for language "multi-language": 20% (building)[CKEditorWebpackPlugin] Error: No JS asset has been found during the compilation. You should add translation assets directly to the application from the 'lang' directory.
Build co

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 23, 2020

These warnings should be fixed now after merging ckeditor/ckeditor5#6654.

@mlewand mlewand self-requested a review April 23, 2020 10:17
Copy link
Contributor

@mlewand mlewand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, works well now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants