[#43509] - [5.2] Replacing Fact::getLang() with Fact::getApp()->getLang()
- Closed
- 21 Jul 2024
- Medium
- Build: 5.2-dev
- # 43509
- Diff
- Hackwar:5.2-factory-getlanguage
- Pending continuous-integration/drone/pr Build is pending Details
User tests: Successful: Unsuccessful:
Summary of Changes
Factory::getLanguage() is deprecated and should not be used anymore. This PR replaces all occurences in our codebase with Factory::getApplication()->getLanguage(). I did this with simple search+replace, so this will need a carefull review (which I'm going to do, too, but more people have to take a look at this.)
Testing Instructions
Codereview.
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin com_categories com_joomlaupdate com_login com_menus com_templates Modules Templates (admin) Repository Front End com_ajax com_banners com_config com_contact com_content com_finder com_users Installation |
| Labels |
Added:
PR-5.2-dev
|
||
| Category | Administration com_admin com_categories com_joomlaupdate com_login com_menus com_templates Modules Templates (admin) Repository Front End com_ajax com_banners com_config com_contact com_content com_finder com_users Installation | ⇒ | Administration com_admin com_associations com_categories com_config com_contact com_content com_contenthistory com_fields com_finder com_installer com_joomlaupdate |
| Title |
|
||||||
Doesn't like it for many cases, you swap just the language dependency into an application dependency which is worse. Trying to add languageawareinterface where possible should be done before falling back to the application object.
I understand and pretty much agree with you, however that is one of the two proposals in the deprecation notice how to replace this. I prefer this solution over the DI container since the DI container solution doesn't contain automatic type hinting. Basically I prefer the solution which PHPStorm automatically understands. But I'm open for any solution. I just want our core to not contain deprecated code usage anymore until 6.0.
Doesn't like it for many cases, you swap just the language dependency into an application dependency which is worse.
The Language can't live without Application (since 4.x?).
It is what it is.
@Hackwar Be aware that language available only after the application is initialised.
On quick check I do not see such use in PR,
however If there any instace of such call (before App initialisation), then the code will fail.
About failing test, it probably need something like this:
We all agree that this is wrong, so I'm closing this PR again. :-)
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-07-21 07:16:05 |
| Closed_By | ⇒ | Hackwar |
Ok, so, our tests fail with this change, since it requires an application to be started now and that fails. @laoneo do you have an idea for this? Would you have someone who could fix this?