Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix composer autoload from classmap to files #3

Closed
wants to merge 1 commit into from

Conversation

igorw
Copy link

@igorw igorw commented Mar 28, 2013

Use "files" autoloading instead of "classmap". This will make
composer auto-require the file, and allow it to be actually used
properly.

Use "files" autoloading instead of "classmap". This will make
composer auto-require the file, and allow it to be actually used
properly.
@shabbyrobe
Copy link
Member

I'm happy to merge the fix if I can establish why, but can you please supply more details about what doesn't work and why files is a better choice? I don't seem to be able to replicate the problem.

I create the following composer.json:

{
    "require": {
        "docopt/docopt": "0.6.*"
    }
}

Then the following PHP:

<?php
require __DIR__.'/vendor/autoload.php';
var_dump(class_exists('Docopt\Handler', false));
$h = new Docopt\Handler();
var_dump(class_exists('Docopt\Handler', false));

The output is this:

bool(false)
bool(true)

@igorw
Copy link
Author

igorw commented Mar 31, 2013

Classmap only works for classes. Since the file also contains functions, files will make sure those functions can be used as well, without requiring the class to be instantiated first.

In particular the Docopt\docopt() function.

@shabbyrobe
Copy link
Member

Ahh, cheers, I understand now. Thanks for reporting. The files autoloader requires the docopt file on every request, which I don't think is a good fit for projects which share a vendor directory between a web app and a CLI.

I think the interim solution is to recommend that you use the Docopt\Handler class with composer, and the long-term solution is to clean up the API so that Docopt\docopt() becomes a static function of a class, thus allowing autoloaders to work properly.

I'm not overly fond of the verbosity of Docopt\Handler::docopt(), but I'd like to minimise the BC breaks as much as I can. At the moment, I'm looking at creating the class Docopt in the global namespace and changing Docopt\docopt to Docopt::handle() (Docopt::docopt() throws a wobbly because it thinks it's the constructor). I'll see if I can come up with something better, but that's where I think it should go.

@shabbyrobe shabbyrobe closed this Mar 31, 2013
@igorw
Copy link
Author

igorw commented Mar 31, 2013

It's a known limitation of PHP autoloading that you cannot autoload functions. Which is unfortunate. I still think a function is more correct than a static method, and worth the small penalty of loading the file.

But the static method solution works as well, so if you want to go with that, that's fine with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants