Skip to content

Conversation

@jmikola
Copy link

@jmikola jmikola commented Dec 22, 2010

See commit message. This solves a problem that the FOS\UserBundle had with the Doctrine extensions loading their mapping files, due to the "FOS" vendor segment in their namespace/directory path.

I believe this is an intuitive solution for the application developer, as it removes the necessity to define each vendor explicitly in the kernel's registerBundleDirs() method.

I'm not sure if FrameworkBundle or others have similar problems looking up directors for vendor-organized bundles, but perhaps this code could be useful outside of just the Doctrine extensions.

…rom vendor-organized bundles

Bundle best practices advocate organizing a bundle under a vendor namespace (e.g. "Bundle\Vendor\VendorBundle").  That puts the onus on the application developer to explicitly add "Bundle\Vendor" to the kernel's bundle directory configuration.

Alternatively, when trying to find a directory for a bundle's namespace we can exhaustively check if parent namespaces are defined in the kernel configuration, which this patch implements.
@fabpot
Copy link
Owner

fabpot commented Dec 22, 2010

Can you give an example of what it simplifies/solves?

@jmikola
Copy link
Author

jmikola commented Dec 22, 2010

The bundle best practices suggest sub-directories for the vendor and category of the bundle. The FOS UserBundle employs this; however, the Doctrine extensions would require the specific vendor/category directory to be defined in the kernel's registerBundleDirs() array. This patch allows you to keep just "Bundle" in that array, and if you have a bundle within "Bundle\FOS", it can still be detected.

I think it's a bit much to require the user to register each vendor/category directory in registerBundleDirs() otherwise, no?

@jwage
Copy link

jwage commented Dec 27, 2010

I am not sure this should be in the DI extension, instead if should be in your Kernel. You can write some code to easily dynamically add all the directories. I think it is better to have less code in Symfony2 for us to maintain and to be more explicit in the configuration side.

public function registerBundleDirs()
{
    $dirs = array();
    $bundles = glob(__DIR__.'/../src/Bundle/*');
    foreach ($bundles as $dir) {
        $info = pathinfo($dir);
        $dirs['Bundle\\'.$info['filename']] = $dir;
    }
    return $dirs;
}

@jmikola
Copy link
Author

jmikola commented Dec 27, 2010

If my own bundle needed such a thing, I'd still do it right in the extension. Otherwise, the best solution is just to make in clear in the documentation that any directory path holding a bundle (be it src/Bundle, /src/Bundle/Vendoror even /src/Bundle/Vendor/Category) must be explicitly defined in registerBundleDirs(). That was my alternative solution to get FOS\UserBundle working while this pull request sat in limbo.

I wouldn't encourage anyone to use your snippet above, though. Adding all sub-directories like that will help bundles that are grouped by only a vendor path. It does nothing for categorized, vendor-grouped bundles, and will likely create garbage paths for non-vendor-named bundles (e.g. src/Bundle/FooBundle).

@beberlei
Copy link

This patch sort of conflicts with pull request 264, i can integrate parts of it but automatic matching will be deprecated in favor of an explicit approach anyways as discussed in the Symfony IRC meeting 3 weeks ago.

I can do more work on it this week and finalize it.

@lsmith77
Copy link

yeah ... 264 seems to be the way to do things down the road.

@jmikola
Copy link
Author

jmikola commented Dec 27, 2010

I wish I had seen 264 before doing this :)

I agree that it's a better approach; closing this one.

This pull request was closed.
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.

5 participants