joomla-cms
joomla-cms copied to clipboard
[5.2] Replacing Fact::getLang() with Fact::getApp()->getLang()
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:
-
[X] No documentation changes for docs.joomla.org needed
-
[ ] Pull Request link for manual.joomla.org:
-
[X] No documentation changes for manual.joomla.org needed
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?
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: https://github.com/joomla/joomla-cms/blob/05b7573db8a7001ba85c1cbdddc43500d68ad4dd/tests/Unit/Plugin/EditorsXtd/PageBreak/Extension/PageBreakTest.php#L47-L48
We all agree that this is wrong, so I'm closing this PR again. :-)