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

Feature/main/i6328 improved localization #7336

Merged
merged 20 commits into from
Feb 18, 2022

Conversation

jonasraoni
Copy link
Contributor

@jonasraoni jonasraoni commented Sep 25, 2021

Description

  • Instead of registering a specific file (e.g. plugin/locale/en/common.po), the code is now expecting a folder (e.g. plugin/locale), which should follow a single rule: its direct sub-folders should refer to locale names (e.g. ./en_US, ./pt_BR, etc.).
  • The application keeps track of the paths, but doesn't load anything until a translation is requested.
  • When a translation for a specific locale is requested, it tries to find a LocaleBundle (a merged collection with all the translation files found for the given locale), if not found, it will build one.

LocaleBundle

  • When a LocaleBundle is requested, the Locale class will pass through the registered folders (it's also possible to register a callback/dynamic file loader), access the sub-folder related to the requested locale (e.g. ./en_US), and retrieve the available .po files recursively. These files will be sent to the newly created LocaleBundle.

Translation

  • When a translation is requested, the LocaleBundle will attempt to find the translations (merged .po files) in the disk/cache. The cache key is built using the path + modified date of its locale files + expiration interval. This way, any modification in the expiration, order of files, paths or modification dates will invalidate the cache.
  • Since a translation might be requested before the application and all the plugins registered their folders, every different combination of available folders, at the time of the request, will issue a new cache (in my OJS installation, after installing/running some things, I've ended up with 8 cached files for the en_US bundle, each one with around ~450KB).
  • If there's no cache, every .po file of the bundle will have to be parsed, transformed into an array, and have its translations merged sequentially into the bundle (there's an optional priority/order argument). This process also has an individual cache for each locale file (invalidated by the modification date).

Merging physically the locale files into a single one would also give a little performance bump.

classes/cliTool/CommandLineTool.inc.php Show resolved Hide resolved
classes/core/PKPApplication.inc.php Outdated Show resolved Hide resolved
classes/core/PKPString.inc.php Outdated Show resolved Hide resolved
classes/site/VersionCheck.inc.php Show resolved Hide resolved
classes/xml/PKPXMLParser.inc.php Show resolved Hide resolved
classes/template/PKPTemplateManager.inc.php Show resolved Hide resolved
dtd/locales.dtd Show resolved Hide resolved
includes/functions.inc.php Show resolved Hide resolved
templates/install/install.tpl Outdated Show resolved Hide resolved
tests/classes/i18n/LocaleTest.php Show resolved Hide resolved
@asmecher
Copy link
Member

asmecher commented Oct 7, 2021

@jonasraoni, there are lots of useful clean-ups here, but it's impossible to review the code thoroughly when they're mixed together; could you separate them into individual commits? For example,

  • PHP5 compatibility code / shims for missing functions removed
  • Constants moved into classes
  • Introduced functions in place of named constants for upgrade/install status
  • etc.

@asmecher
Copy link
Member

asmecher commented Oct 7, 2021

One more request: could you document the changes that developers using older versions of OJS will need to know in adapting their code to this? It'll give me something to think about while reviewing, plus @NateWr can collect this for his release notebook. See #6091 (comment) for an example.

@jonasraoni
Copy link
Contributor Author

Ok! I think I'll create new issues for these extra modifications.

@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch 2 times, most recently from 5edc1ee to 5fd4a99 Compare October 17, 2021 20:18
@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch 5 times, most recently from 86194ef to 4148b82 Compare October 26, 2021 12:47
@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch from 4148b82 to f39c880 Compare November 7, 2021 18:14
@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch from f39c880 to ab2c3b2 Compare November 15, 2021 07:14
@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch 10 times, most recently from 2673893 to df02b8b Compare November 30, 2021 20:41
@jonasraoni jonasraoni force-pushed the feature/main/i6328-improved-localization branch 5 times, most recently from fa20575 to 90f3703 Compare February 17, 2022 07:23
jonasraoni and others added 20 commits February 17, 2022 23:19
- The Laravel container is always initialized
- Removed the need of loading specific components by loading all registered locale files
- Integrated with the Laravel translation system and removed workaround patch
- Added a Laravel Facade to access the Locale where DI isn't available
- Simplified the time zone handling (the list is now loaded from PHP)
- Enabled the Laravel caching system and added a OpCache driver (cached *objects* must implement the __set_state)
…ster) and cached the result to reduce disk usage
…ended the current translate tag and modifier
- Update code to replace the locales.xml (added detection of text direction + completeness calculation + integrated ISO Alpha 2/3)
- Implemented an "IsoCodes" translation driver to replace the default driver, the GettextExtensionDriver doesn't work very well and the SymfonyTranslationDriver has a memory/performance issue.
…rnal plugins (PHP 8 emits fatal error when accessing missing constants)
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.

5 participants