[ticket/13762] Moving language related functionality into a service #3535
Conversation
|
Replacing Anyway I'd like to keep the BC with the old |
|
Just reverted that commit while you were typing that message. BC definitely should be kept and my RegEx replace turned out pretty badly anyhow. |
| @@ -116,6 +116,12 @@ services: | |||
| - @controller.resolver | |||
| - @request_stack | |||
|
|
|||
| language: | |||
There was a problem hiding this comment.
This service part should be rewritten, but I'm not sure how exactly.
Probably this part should be renamed to language.parent, made abstract, and then language service could link to it, and we could specify passing extension manager into the setter, but that should be environment dependent, so I'm not sure where it should be specified.
There was a problem hiding this comment.
I think that we should split it into at least two services:
- language_loader which will find the language files and load them when requested (basically it's a stateless service (minus caching) which as a
load(<component>, <locale>) : array('key' => 'value')method or something similar). - language which will take a loader as parameter. This service will be used by everyone.
There was a problem hiding this comment.
by the way, move this service definition to services_language.yml please
|
Hi, |
| public function get_available_languages() | ||
| { | ||
| // Find available language packages | ||
| $finder = new Finder(); |
There was a problem hiding this comment.
any reason why you don't use phpbb\finder ?
There was a problem hiding this comment.
phpbb\finder seems to lack the functionality I use here, also caching is not an issue here.
There was a problem hiding this comment.
and you want only the core language files, right?
There was a problem hiding this comment.
Mostly. This service used at two places, the installer, and the ACP language install module, it was implemented both places so this is the deduplication of it. And the assumption behind my code (and probably the old one as well) that a language is only installable when the core files are there, therefore I don't really care for extension language packs here.
There was a problem hiding this comment.
to be precise, each time we want to limit the depth we write the iterator chain manually
There was a problem hiding this comment.
with our finder yes, but not with your PR :)
|
https://github.com/phpbb/phpbb/blob/3.1.x/phpBB/install/index.php#L243 needs to be updated |
|
https://github.com/phpbb/phpbb/blob/3.1.x/tests/template/template_test_case.php#L124 you need to update the loader to set the variables (you can use the PHP reflection API for that) |
|
same here: https://github.com/phpbb/phpbb/blob/3.1.x/tests/user/lang_test.php#L21 (and a few lines below) |
|
please rebase |
|
merged #3539 you can rebase |
| namespace phpbb\language\exception; | ||
|
|
||
| /** | ||
| * Exception thrown by \phpbb\language\language::get_plural_form() |
There was a problem hiding this comment.
explain what mean this exception and not which function throw it
| */ | ||
| protected function load_common_language_files() | ||
| { | ||
| foreach ($this->common_language_files as $lang_file) |
There was a problem hiding this comment.
wrap it under if(!$this->common_language_files_loaded )
CHItA
force-pushed
the
ticket/13762
branch
5 times, most recently
from
2d9c982
to
18be1d7
Compare
CHItA
force-pushed
the
ticket/13762
branch
2 times, most recently
from
0b493ee
to
861311e
Compare
| * @param $number int|float The number we want to get the plural case for. Float numbers are floored. | ||
| * @return int The plural-case we need to use for the number plural-rule combination | ||
| * | ||
| * @deprecated 3.2.0-dev (To be removed: 3.3.0) |
There was a problem hiding this comment.
Shouldn't it be "To be removed: 3.4.0"? (or 4.0.0)
Same for all other occurrences below.
There was a problem hiding this comment.
I'm not even sure if anyone uses this function (as it was called by user::lang()). So 3.3.0 is more then too much time to adopt.
[ticket/13762] Moving language related functionality into a service
PHPBB3-13762