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

autoloader fails on case-sensitive file systems #3

Open
ghost opened this issue May 28, 2010 · 4 comments
Open

autoloader fails on case-sensitive file systems #3

ghost opened this issue May 28, 2010 · 4 comments
Labels

Comments

@ghost
Copy link

ghost commented May 28, 2010

I propose creating $filename like so in k_autoload:

$filename = preg_replace(
  '/([A-Za-z0-9]+)_/e',
  "strtolower('$1').'/'", 
  $classname
) . '.php';

The existing autoloader makes everything lower-case, and this instead leaves the last part of the underscore-separated class path alone: "Path_To_SomeClass" becomes "path/to/SomeClass.php".

@lsolesen
Copy link
Collaborator

How about this? I agree that the standard autoloader should not make assumptions on how the developer wants to name classes.

@troelskn
Copy link
Owner

The autoloader will make assumptions in any case. If you don't lowercase, then you assume that files are named in the same fashion as the are referred in code.

Granted, the de-facto standard (PEAR and ZF inclusive) use a mixed case for file names. I have used all lower case for a long time, because class names are case insensitive in PHP, thus there is no guarantee that a user has typed a class name in the same case in every place. It's not that I want to impose any particular style really - Konstrukt it self doesn't rely on autoload functionality, so perhaps the right thing to do would be to simply remove the autoloader from the library and leave it to user land to implement this functionality.

@lsolesen
Copy link
Collaborator

It is difficult, I think. If you include the autoloader, I think it should work according to the standard (PEAR and ZF). Otherwise it will confuse too many users.

@troelskn
Copy link
Owner

I think I concur.

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

No branches or pull requests

2 participants