Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Translation refactoring #1254

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Translation refactoring #1254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions includes/basics.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,9 @@
$database->initializeConfig($kga);

// ============ setup translation object ============
$service = new Kimai_Translation_Service();
Kimai_Registry::setTranslation(
$service->load(
$kga->getLanguage()
)
);
unset($service);
$translationService = new Kimai_Translation_Service();
$translationService->load();
unset($translationService);

$tmpDir = WEBROOT . 'temporary/';
if (!file_exists($tmpDir) || !is_dir($tmpDir) || !is_writable($tmpDir)) {
Expand Down
9 changes: 8 additions & 1 deletion libraries/Kimai/Registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public static function setConfig(Kimai_Config $config)
* Return the global configuration, merged with all user related configurations.
*
* @return Kimai_Config
* @throws \Zend_Exception
*/
public static function getConfig()
{
Expand All @@ -46,6 +47,7 @@ public static function getConfig()
* Returns the database layer to use.
*
* @return Kimai_Database_Mysql
* @throws \Zend_Exception
*/
public static function getDatabase()
{
Expand Down Expand Up @@ -74,6 +76,7 @@ public static function setUser(Kimai_User $user)

/**
* @return Kimai_User
* @throws \Zend_Exception
*/
public static function getUser()
{
Expand Down Expand Up @@ -101,7 +104,7 @@ public static function getCache()
{
return self::get('Zend_Cache');
}

/**
* @param Kimai_Auth_Abstract $authenticator
*/
Expand All @@ -112,6 +115,7 @@ public static function setAuthenticator(Kimai_Auth_Abstract $authenticator)

/**
* @return Kimai_Auth_Abstract
* @throws \Zend_Exception
*/
public static function getAuthenticator()
{
Expand All @@ -120,6 +124,8 @@ public static function getAuthenticator()

/**
* @param Kimai_Translation_Data $translation
*
* @throws \Zend_Exception
*/
public static function setTranslation(Kimai_Translation_Data $translation)
{
Expand All @@ -129,6 +135,7 @@ public static function setTranslation(Kimai_Translation_Data $translation)

/**
* @return Kimai_Translation_Data
* @throws \Zend_Exception
*/
public static function getTranslation()
{
Expand Down
30 changes: 16 additions & 14 deletions libraries/Kimai/Translation/Data.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,41 +31,43 @@ class Kimai_Translation_Data extends \ArrayObject
* Create a translation object:
* pre-fill with english and replace by $language specific data.
*
* @param array|null|object $language
* @param array|string|object $language
*
* @throws \Exception
*/
public function __construct($language)
{
$default = Kimai_Config::getDefault(Kimai_Config::DEFAULT_LANGUAGE);
$data = include WEBROOT . 'language/'.$default.'.php';
$data = include WEBROOT . 'language/' . $default . '.php';
parent::__construct($data, \ArrayObject::ARRAY_AS_PROPS);
$this->addTranslations($language);
}

/**
* Adds the translations for the given language.
*
* @param $language
* @throws Exception
* @param string $language
* @param string $path
*/
public function addTranslations($language)
public function addTranslations($language, $path = null)
{
// no need to load the default or already requested language again!
$default = Kimai_Config::getDefault(Kimai_Config::DEFAULT_LANGUAGE);
if (empty($language) || $language == $default || $language == $this->language) {
return;
if ($path === null) {
// no need to load the default or already requested language again!
$default = Kimai_Config::getDefault(Kimai_Config::DEFAULT_LANGUAGE);
if (empty($language) || $language == $default || $language == $this->language) {
return;
}
$path = WEBROOT;
}

$languageFile = WEBROOT . 'language/'.$language.'.php';
$languageFile = $path . 'language/' . $language . '.php';
if (!file_exists($languageFile)) {
Kimai_Logger::logfile('Requested translation is missing: ' . $language);
return;
}

$this->language = $language;
$data = array_replace_recursive(
$this->getArrayCopy(),
include $languageFile
);
$data = array_replace_recursive($this->getArrayCopy(), include $languageFile);

$this->exchangeArray($data);
}
Expand Down
25 changes: 13 additions & 12 deletions libraries/Kimai/Translation/Service.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
class Kimai_Translation_Service
{

/**
* Load a translation data.
*
* @throws \Exception
*/
public function load()
{
$language = Kimai_Registry::getConfig()->getLanguage();
Copy link
Member

@kevinpapst kevinpapst Sep 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest, I think this is a breach of the responsibility principle.
From an architectural point of view the translation service should neither know nor care about a global registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any better idea? the way how it was before is definitely not better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm loading my own translations with that:

$this->translationData = Kimai_Registry::getTranslation();
$this->translationData->addTranslations('en', __DIR__ . '/');
$this->translationData->addTranslations('de', __DIR__ . '/');
Kimai_Registry::setTranslation($this->translationData);

if you have a better idea, I'm open for feedback ;)

Copy link
Member

Choose a reason for hiding this comment

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

So, you are doing it like it was done before, but you find the way before not better?
Makes not much sense to me 😈
Yes my proposal stands: leave as it was before. There is absolutely no reason why the translation service should know about the registry. Only the registry is global and knows about all objects it handles.
Don't be offended - but I don't really care about Kimai 1. The code is crap and won't get better by any refactoring. If you like to move stuff around: do so!

$translationData = new Kimai_Translation_Data($language);
Kimai_Registry::setTranslation($translationData);
}

/**
* Returns an array of all language codes.
*
Expand All @@ -34,21 +46,10 @@ public static function getAvailableLanguages()
{
$languages = [];
foreach (glob(WEBROOT . 'language/*.php') as $langFile) {
$languages[] = str_replace(".php", "", basename($langFile));
$languages[] = str_replace('.php', '', basename($langFile));
}
sort($languages);

return $languages;
}

/**
* Load a translation data.
*
* @param $name
* @return Kimai_Translation_Data
*/
public function load($name)
{
return new Kimai_Translation_Data($name);
}
}