Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[HttpKernel][Bundle] Bundle multiple Inheritance #8549

Closed
wants to merge 2 commits into from

7 participants

@wpottier

add support of array for Bundle::getParent so bundle could override multiple parents

Q A
Bug fix? no
New feature? yes
BC breaks? yes (as Stof note it : returning an array breaks any bundle relying on this method currently)
Deprecations? no
Tests pass? yes (same error on 5.3.3 travis build as master repository)
Fixed tickets #8071
License MIT
Doc PR -
  • submit changes to the documentation
  • submit changes to the sension/framework-extra-bundle to allow Sensio\Bundle\FrameworkExtraBundle\Templating\TemplateGuesser::guessTemplateName to work with array returned by Bundle::getParent
@masev

+1 useful feature for many use case !

@lsmith77
Collaborator

i remember back then we discussed this feature and decided against it.

@stof stof commented on the diff
src/Symfony/Component/HttpKernel/Bundle/Bundle.php
@@ -135,7 +135,7 @@ public function getPath()
/**
* Returns the bundle parent name.
*
- * @return string The Bundle parent name it overrides or null if no parent
+ * @return string|array The Bundle parent name it overrides or null if no parent; Use array of parent name if bundle override more than one parent
@stof Collaborator
stof added a note

Changing the signature of a method in the public API is a BC break (returning an array breaks any bundle relying on this method currently)

yes you right; I have missed that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabpot
Owner

As @lsmith77 mentioned, this feature has already been rejected in the past.

Can you elaborate a bit more about some real-world use cases?

@fabpot fabpot closed this
@ollietb

I have a real world use-case:
I have 2 bundles which are using mostly the same code as each other, however they are using different designs so a lot of the template code across the site needs to change. One bundle extends the other, but there are templates in some vendor bundles which also need to be overwritten (eg FOSUserBundle, MopaBootstrapBundle). I can't overwrite the templates in more than one bundle so I'm pretty stuck. Multiple bundle inheritance would help me loads.
I can't really think of a use case for another bundle needing to reference another bundle's getParent method.

@mvrhov

Just put the overrides into app/Resources/< bundlename >

@wpottier

It's not really the same use case.
If you put your overrides in app/Resources you will not be able to package these overrides with your bundle to other projects.

@ollietb

@mvrhov I can't because my 2 bundles need to override the vendor bundles differently. app/Resources only seems to override for all local bundles.

If we can't change the return type for getParent(), how about adding a new method getParents() that can return an array?

@ollietb

I suppose the crux of the issue is that I have a local bundle, call it BundleOne, which already overrides some vendor bundle files in app /Resources. I've created a new local bundle, BundleTwo, which needs to also override some of the same files in app/Resources but with some changes. This can't be done unless I can move the required app/Resources overrides into BundleOne and BundleTwo which will require multiple inheritance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  src/Symfony/Component/HttpKernel/Bundle/Bundle.php
@@ -135,7 +135,7 @@ public function getPath()
/**
* Returns the bundle parent name.
*
- * @return string The Bundle parent name it overrides or null if no parent
+ * @return string|array The Bundle parent name it overrides or null if no parent; Use array of parent name if bundle override more than one parent
@stof Collaborator
stof added a note

Changing the signature of a method in the public API is a BC break (returning an array breaks any bundle relying on this method currently)

yes you right; I have missed that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
*
* @api
*/
View
2  src/Symfony/Component/HttpKernel/Bundle/BundleInterface.php
@@ -65,7 +65,7 @@ public function getContainerExtension();
* between the bundles, just a way to extend and override an existing
* bundle.
*
- * @return string The Bundle name it overrides or null if no parent
+ * @return string|array The Bundle parent name it overrides or null if no parent; Use array of parent name if bundle override more than one parent
*
* @api
*/
View
20 src/Symfony/Component/HttpKernel/Kernel.php
@@ -485,14 +485,22 @@ protected function initializeBundles()
}
$this->bundles[$name] = $bundle;
- if ($parentName = $bundle->getParent()) {
- if (isset($directChildren[$parentName])) {
- throw new \LogicException(sprintf('Bundle "%s" is directly extended by two bundles "%s" and "%s".', $parentName, $name, $directChildren[$parentName]));
+ if ($parentNames = $bundle->getParent()) {
+
+ if (!is_array($parentNames)) {
+ $parentNames = array($parentNames);
}
- if ($parentName == $name) {
- throw new \LogicException(sprintf('Bundle "%s" can not extend itself.', $name));
+
+ foreach ($parentNames as $parentName) {
+
+ if (isset($directChildren[$parentName])) {
+ throw new \LogicException(sprintf('Bundle "%s" is directly extended by two bundles "%s" and "%s".', $parentName, $name, $directChildren[$parentName]));
+ }
+ if ($parentName == $name) {
+ throw new \LogicException(sprintf('Bundle "%s" can not extend itself.', $name));
+ }
+ $directChildren[$parentName] = $name;
}
- $directChildren[$parentName] = $name;
} else {
$topMostBundles[$name] = $bundle;
}
Something went wrong with that request. Please try again.