Namespaces support for JLoader#1591
Namespaces support for JLoader#1591LouisLandry merged 3 commits intojoomla:stagingfrom florianv:jloader-namespaces
Conversation
|
@florianv would you please migrate the documentation in the description from #1400 over to this one so we have it all here? The change log is generated based on merged pull requests so it's nice to have it all in one place. Also, I wonder if you'd entertain a thought I've had on // Setup the Joomla Platform autoloader.
JLoader::setup($enableJimport, $enableNamespaces);The idea being that those two boolean flags would decide whether we even add the different autoloader methods. For now we would need to default the first argument to |
tests/suites/unit/JLoaderTest.php
Outdated
There was a problem hiding this comment.
These should be broken up into different test methods so you can get a granular picture of what is breaking.
|
Yes it is a good idea :) What I was thinking before is eventually to have 2 loaders : JLoader and JLoaderNamespace having the same base. <?php
interface JLoaderBase
{
public function load($class);
public function setup();
// Something = prefix / namespace
public function register($something, $path, $reset = false);
}So people can set up the namespace one if they want to independently of the actual one. Both solutions are correct I think, but yours implies less changes. But, supposing in the future Joomla is fully namespaced and wants to get rid of the actual loader, I think maybe we would like not having param refering to the old one in setup(). Edit : But my solution is only valid if the loader keeps his name : JLoaderNamespace, when JLoader leaves. Otherwise people would have to modify their calls to it. |
|
@ianmacl Thanks, I will split them asap. |
|
@florianv I think your solution is probably the more "academically correct" one. That being said, the class is static and the way it is currently used I am not sure that I think there'd be much benefit in changing that behavior. As for the parameter ordering I actually think I agree with you. How about we do something like: <?php
/**
* @package Joomla.Platform
*
* @copyright Copyright (C) 2005 - 2012 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE
*/
defined('JPATH_PLATFORM') or die;
/**
* Static class to handle loading of libraries.
*
* @package Joomla.Platform
* @since 11.1
*/
abstract class JLoader
{
// ...
/**
* Method to setup the autoloaders for the Joomla Platform. Since the SPL autoloaders are
* called in a queue we will add our explicit, class-registration based loader first, then
* fall back on the autoloader based on conventions. This will allow people to register a
* class in a specific location and override platform libraries as was previously possible.
*
* @param boolean $allowMixedCasePaths True to allow class paths to use mixed case paths that match class names.
* @param boolean $enableNamespaces True to enable PHP namespace based class autoloading.
* @param boolean $enableJimport True to enable legacy jimport() based library loading.
*
* @return void
*
* @since 11.3
*/
public static function setup($allowMixedCasePaths = false, $enableNamespaces = false, $enableJimport = true)
// ...
}
// Setup the Joomla Platform autoloader.
JLoader::setup($enableJimport, $enableNamespaces);This has the advantage of not forcing a change in method signature down the road. At some point the The one we end up keeping is the flag to allow the mixed case paths. I actually like that you have that option in there, but I'd rather not incur the cost of an extra filestat on every load if it is just completely unnecessary and all paths are lower case. Does that make sense? |
|
Yes it does. The $allowMixedCasePaths is important. I don't know if you have planned about having the Joomla directories and files names camel cased, that's why I did the two options. I think there are a few advantages using camel cased directories and files : |
|
I wouldn't say we have plans one way or another, but we do plan to explore our options. I like the possibility of using it regardless of the path we take for the core platform libraries going forward. |
|
Ok. Maybe 3 options are needed just in case : 1- Lower case 2- Mixed case 3- Camel case In the future, if we want the best speed, we can use maps : an array of keys containing namespace names and values the full path to it, so the loading is direct. |
|
So, I went ahead with the three options and updated the setup method. Then, I played around and tried various methods, and it seems that my first implementation was not the fastest one. I also removed the cost of the class_exists in the namespace load functions and used include_once instead. In a nutshell, it is three times faster on my machine. I also decided to split the three loaders in three separated methods. I also complemented the test with an other class JLoaderNamespaceTest to test that it really works when the class names are passed to the auto load function by PHP. I thought they were a leading '' but it is not the case. It also splitted the tests as @ianmacl said. I fixed a few editor warnings in the other load methods. One @throws missed, and some other were declared void but returned a boolean. |
|
@florianv would you mind taking a copy of this file: and making a gist or similar for with with additional documentation explaining the new namespace support and how developers can use it. We can merge it later when we redo the docs. Thanks in advance. |
|
@eddieajau Do you want me to replace the actual loader.xml docbook by a markdown file, merge the additions and include it in the commit ? If I understand the docs will be translated to markdown files ? Thanks. |
|
No, don't replace it. A gist will be fine for now. |
|
Here is the gist : https://gist.github.com/fa4ea8ffc7b3b34151b2 |
|
Thanks. We've just converted all the docs over to markdown so just update |
|
I have rearranged a few things in the doc |
|
This is great @florianv. I'm going to give you one last nit-picky thing then I promise I'm going to merge it. Would you please add |
|
Thanks. Yes indeed it is clearer. I don't squash the commits so you don't have to read the whole diff. |
|
Awesome @florianv, you are the very model of a fantastic contributor... thank you! |
Namespaces support for JLoader
|
Awe yeah! |
This pull request adds support for class loading and autoloading based on namespaces.
If your class is located in :
BASE_PATH/Chess/Piece/Pawn.php:You can register it to the auto loader, by registering the ROOT namespace
Chess:JLoader::registerNamespace('Chess', BASE_PATH . '/Chess');All classes respecting the naming convention in a given namespace :
namespace Folder\SubFolder;for classes inBASE_PATH/Folder/SubFolder/will be autoloaded.If you have lower case directory names and class names
BASE_PATH/folder/subfolder/, you can either declarate the namespace with lower or camel cases, and it will work too.But note that the first param of JLoader::registerNamespace is case sensitive and must match the namespace declaration case.
Examples:
namespace Chess;JLoader::registerNamespace('Chess', PATH_TO_CHESS);namespace chess;JLoader::registerNamespace('chess', PATH_TO_CHESS);You can also register multiple lookup paths for a given namespace (like the prefix).