User tests: Successful: Unsuccessful:
Renames the translate function to something less generic to avoid potential conflicts with existing plugins. If there are better names feel free to comment, because for me is translate the right name.
Ping @roland-d
Create a GET request task and run it manually in the back end from the web interface.
All works.
All works.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries Front End Plugins Unit Tests |
Also whilst we're reviewing this function is there a reason this method uses PHP's sprintf instead of the \Joomla\CMS\Language\Text::sprintf function
How about just doing it correctly. Revert the - untested change which was clearly wrong and then you can address the issue at leisure
Please can we just keep to
_
. It's also similar to wordpress/kohana/cakephp (which use double underscore) which gives us some parity with other similar platforms
I was also thinking about this, but we are using also ::_ in Route, so my thought was that in a plugin it would be a bit strange to use underscore for translation. But I'm open here really.
Also whilst we're reviewing this function is there a reason this method uses PHP's sprintf instead of the \Joomla\CMS\Language\Text::sprintf function
What we can also do is to move sprintf to the language. The target I want is to be able to avoid Text::_
for the sake of testing. When the language is directly used for translation, then we can properly unit testing it without mocking Factory::$application
.
Can we please remove this function, I see 0 benefit. It only provides a subset of the functions from jlanguage. And pollutes the object.
As said in my comment before. It allows us to mock the application and then to write correct unit tests for plugins, which I don't think I have to explain here the benefits. Additionally it lowers the dependency to the Factory
class. As I said, another solution would be to add the sprintf function to the language class, then the translate function can be removed.
When you have a look on the plugin unit tests for example here https://github.com/joomla/joomla-cms/blob/4.2-dev/tests/Unit/Plugin/Task/Requests/Extension/RequestsPluginTest.php#L99 or in #38072, then you will see the translate function allows us to write unit tests for plugins. Because a plugin should be testable as they should contain atomic functionality.
The please inject the language object if needed by the Plugin. I think the service provider should detect all interfaces implemented inject it.
There is no interface for the language and the language is available already through the application. Either way we keep this function or do copy the sprintf function from the Text
class to the language object.
My understanding is that application should not be available to the plugin at some point in time
I think the plugin will always have a reference to the application as they act globally in the core. But they should not load it by itself, instead of, it should be injected. There we use for transition the app from the Factory
class.
I made an alternative pr #38203 which reverts the translate function and goes directly to the application object.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-07-02 18:19:18 |
Closed_By | ⇒ | laoneo | |
Labels |
Added:
?
?
|
Please can we just keep to
_
. It's also similar to wordpress/kohana/cakephp (which use double underscore) which gives us some parity with other similar platforms