Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

Feature rewrite #14

Merged
merged 29 commits into from
Nov 10, 2019
Merged

Feature rewrite #14

merged 29 commits into from
Nov 10, 2019

Conversation

lcharette
Copy link
Member

@lcharette lcharette commented Oct 31, 2019

This rewrite includes :

The idea here is the Translator accept a Dictionary, which accept a Locale :

Untitled Diagram

In code, this looks like :

$locale = new Locale('fr_FR');
$dictionary = new Dictionary($locale, $this->ci->locator);
$translator = new Translator($dictionary);

The locale contains all the info about the locale itself, including all metadata loaded from the config file. Speaking of the config file, by default, the file is loaded from locale://fr_FR/locale.yaml path, but it can be overwritten by the constructor second argument.

The Dictionary returns the data for all available locale files, including the data from dependent locale.

Finally, the translator uses the array from the Dictionary, plus the info from the locale saved inside the Dictionary to provide the actual translation.

So switching the locale (userfrosting/UserFrosting#868) is simply a matter of providing a different Dictionary, based on a different Locale.

It also mean to generate a list of all available locales (userfrosting/UserFrosting#850) we don't need the whole translator. The UF config files can contains only a list of "available" locales

Everything is also constructed using Interfaces, so one could use their own Locale or Dictionary class. For example, a Locale could be loaded from the db, or the dictionary loaded from xml files.


Things left to do :

  • Make the Dictionary extends the Repository class
  • Move the Dictionary repository methods to an abstract class + add tests + extend the core one instead? (What to do with the $items?)
  • Make all param from the config file optional
  • Generate API Doc
  • Update README
  • Update CHANGELOG

@codecov
Copy link

codecov bot commented Oct 31, 2019

Codecov Report

Merging #14 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##             develop    #14   +/-   ##
========================================
  Coverage        100%   100%           
- Complexity       125    151   +26     
========================================
  Files             18     19    +1     
  Lines            205    258   +53     
========================================
+ Hits             205    258   +53
Impacted Files Coverage Δ Complexity Δ
src/Translator.php 100% <100%> (ø) 39 <37> (?)
src/Dictionary.php 100% <100%> (ø) 14 <14> (?)
src/Locale.php 100% <100%> (ø) 20 <20> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c964f2d...9d517f2. Read the comment docs.

@lcharette
Copy link
Member Author

The locale contains all the info about the locale itself, including all metadata loaded from the config file. Speaking of the config file, by default, the file is loaded from locale://fr_FR/locale.yaml path, but it can be overwritten by the constructor second argument.

Actually, the only issue is here. Right now, it assume all dependent locale uses that config file. Not sure if it can be done in another way...

@lcharette
Copy link
Member Author

@lcharette lcharette marked this pull request as ready for review November 10, 2019 00:23
@lcharette lcharette merged commit c6b410d into develop Nov 10, 2019
@lcharette lcharette deleted the feature-rewrite branch November 10, 2019 00:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant