User tests: Successful: Unsuccessful:
Renames the \Joomla\CMS\HTML\Registry
to \Joomla\CMS\HTML\HTMLRegistry
.
Merge on review @wilsonge.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_admin com_content Libraries |
The FQCN (Joomla\CMS\MVC\Controller\BaseController
or Joomla\Http\Response
) IS the meaningful class name. That's kind of my point. The short name (BaseController
or Response
) doesn't necessarily have to be meaningful entirely on its own (look at how the container service providers are named as an example).
If we really want the short name to be a meaningful class name on its own, then we need to rename a bunch more classes than this one Registry
. That's why I'm saying pick a standard and stick with it so we can all adapt accordingly because right now there are mixed uses and we're just going to sit here and argue over whether class names are appropriate without a core standard.
(As I said in #17672 I still think ServiceRegistry
is a more meaningful name than HTMLRegistry
, it's a registry of services for the HTMLHelper::_()
processor (I don't think HTMLHelper
was the "best" class name for this either but too late to do anything on that one), it's not a registry of HTML "things", especially as the entire API has been misused and isn't entirely a set of services creating some kind of HTML output anymore)
A getRegistry
function in a trait is for me too generic. I fear that it will lead sooner or later to clashes. Then I started to rename things and one change lead to the next one and it ended up with a rename to HTMLRegistry
.
getX()
doesn't have to return a class named X
.
To be honest, the intent really wasn't to have that registry accessible as a global service, but as an internal detail to HTMLHelper
. So looking at some of this code for the first time in a while, I feel like some of it actually is complicating matters a bit more than needs to be. Unless you've got a master plan for making HTMLHelper::_()
non-static (which you basically need Laravel style Facades for, that idea has already been proposed and shot down because it's too technically complex for the Joomla user base), I'd honestly back out some of those changes and just have the service layer call HTMLHelper::registerService('foo', $thing);
versus trying to put that in the container.
Actually it would be nice to get rid of HTMLHelper and to have some none static way to load these.
I'm going to be honest here. Our layout rendering engines need to go and need to be replaced with something like Twig or Blade or Plates.
Most of what's used for JHtml helpers really should be a Blade directive or a Twig function/macro or a Plates function/extension. So in the rendering layer you're calling to a registered service of the templating engine, you're not calling to a global service that has been misused and abused.
Granted, with pure PHP we can't prevent layout files from misusing the PHP API, but we really need to make some hard decisions at some point and find a way to move to a proper decoupled rendering solution. The fact that in a component layout file I have direct access to the entire JViewLegacy
API to do anything I want is bad, the layout file should ONLY have access to injected data.
Agree, that a proper template engine should be used instead of our PHP layouts. But we have to deal with what we have now and need to find a way to improve it as it is very unlikely to get rid of it.
But we have to deal with what we have now and need to find a way to improve it as it is very unlikely to get rid of it.
Because every attempt to propose needed changes in the MVC layer gets shot down because of B/C concerns, and every attempt to introduce a new MVC layer in parallel where B/C isn't as major of a concern gets shot down. Sooner or later we're all going to wise up and realize you can't keep monkey patching a broken architecture. Or, we all need to just accept that the architecture is locked in the state it is because Joomla doesn't have the resources to adequately build it out the way it needs to be done.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-05-28 08:29:36 |
Closed_By | ⇒ | laoneo | |
Labels |
Added:
?
|
Again, why? Not much has changed since #17672.
If
Registry
is considered too ambiguous thenJoomla\Registry\Registry
should also be renamed. If the issue is there might be too many classes where the short name isRegistry
, that's a non-issue because namespaces are intended to separate these things.Pick one class naming convention and stick with it. Either we prefix/suffix all class names with something from the previous namespace segment (i.e.
HTML\HTMLRegistry
orController\BaseController
) or we don't (i.e.Service\Provider\Application
orHttp\Response
) but pick one and be consistent throughout the code with it as a convention/standard. Anything short of that is merely nitpicking details for the sake of nitpicking details.