Skip to content
This repository was archived by the owner on Nov 26, 2017. It is now read-only.

Namespace support for JLoader (with tests)#1400

Closed
florianv wants to merge 2 commits intojoomla:stagingfrom
florianv:jloader
Closed

Namespace support for JLoader (with tests)#1400
florianv wants to merge 2 commits intojoomla:stagingfrom
florianv:jloader

Conversation

@florianv
Copy link
Copy Markdown
Contributor

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 :

<?php
namespace Chess\Piece;

class Pawn {} 

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 in BASE_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).

@florianv florianv closed this Aug 2, 2012
@florianv florianv reopened this Aug 6, 2012
@realityking
Copy link
Copy Markdown
Contributor

Wouldn't it be better to add a PSR-0 (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md) mode?

@florianv
Copy link
Copy Markdown
Contributor Author

florianv commented Aug 6, 2012

Hello, it works like that.

Each namespace must have a top-level namespace ("Vendor Name").
Each namespace can have as many sub-namespaces as it wishes.
Each namespace separator is converted to a DIRECTORY_SEPARATOR when loading from the file system.

The registerNamespace is used to register the path to the root namespace : /path/to/project/lib/vendor/ in the example.
But you can register multiple paths for a given namespace.
I think it can be useful. But I can remove it.

"Alphabetic characters in vendor names, namespaces, and class names may be of any combination of lower case and upper case."

For this, I'm not sure if file_exists and include are case sensitive (if it depends on the system).
That's why I added two checks : it allows to have a namespace declaration containing upper case letters and lower case directory and file names.
For example in Joomla : namespace Joomla\Model; class Base {} can correspond to a file in joomla/model/base.php or Joomla/Model/Base.php.

@florianv
Copy link
Copy Markdown
Contributor Author

florianv commented Sep 2, 2012

Just an up. If you look at this PSR-0 Loader https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassLoader.php, they use an array for namespaces (same namespace possible for different paths) too and it looks like the one in this pull request.

The difference is that you can declarate namespaces camelcased and have lower case or camelcased directory and file names.
I find it useful because people might be interested in having camel cased files in their apps, and if Joomla is namespaced its files can stay lower case.

Also it ensures it will work properly on case sensitive and insensitive systems.

@dongilbert
Copy link
Copy Markdown
Contributor

So the comment I added earlier, did it get deleted? Or maybe it's an issue with github, since the site has been experiencing problems recently.

@LouisLandry
Copy link
Copy Markdown
Contributor

I got the email for your comment, but never saw it online. It was right around the time they started experiencing database issues, so perhaps it's being missing is related. Maybe you are the one who broke GitHub ;-)

@LouisLandry
Copy link
Copy Markdown
Contributor

@florianv this pull request isn't mergeable any longer. Would you mind rebasing it (and maybe even squashing the two commits into one) so we can finish the discussion about it. I'm inclined to merge it, but I want to play with it a little bit and see what the overall impact on performance may look like. I really love the initiative in doing this though.

I'm going to close the pull request for now. Once you get it rebased (double check code styling please) please re-open it so we can finish the conversation. Thanks a bunch!

@LouisLandry LouisLandry closed this Oct 9, 2012
@florianv
Copy link
Copy Markdown
Contributor Author

florianv commented Oct 9, 2012

Ok, I will do it asap.
I want to insist that you can only register root namespaces like 'Joomla', but not 'Joomla\Cms' for example.
You can register the cms classes by adding a new path for the 'Joomla' namespace but not a path to 'Joomla\Cms' which is a sub namespace of Joomla.
So if people want to use a package from an other framework, let's say Doctrine\ORM, they will have to put it in a Doctrine folder and register the 'Doctrine' root namespace pointing to that folder.

This is because I take only the first part of the namespace to avoid iterating the whole namespaces stack.

@LouisLandry
Copy link
Copy Markdown
Contributor

I'm quite comfortable with that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants