make imagine optional even when using the image form type #45

Merged
merged 1 commit into from Mar 19, 2013

Conversation

Projects
None yet
3 participants
Member

dbu commented Mar 15, 2013

i noticed that the image form type could be useful even without having imagine. this is a slight bc break on the configuration but i think its worth it.

would update the documentation as well if we merge this

dbu added a commit that referenced this pull request Mar 19, 2013

Merge pull request #45 from doctrine/make-imagine-optional
make imagine optional even when using the image form type

@dbu dbu merged commit 693719e into master Mar 19, 2013

@dbu dbu deleted the make-imagine-optional branch Mar 19, 2013

+ : array();
+ $container->setParameter('doctrine_phpcr.odm.subscriber.image_cache.filter', $filter);
+ $container->setParameter('doctrine_phpcr.odm.subscriber.image_cache.all_filters', $filters);
+ $this->loader->load('odm_image.xml');
@lsmith77

lsmith77 Mar 19, 2013

Member

you are still loading this always which means you always need LiipImagineBundle due to the liip_imagine.cache.manager dependency in doctrine_phpcr.odm.subscriber.image_cache.

now to solve this we can either check for the availability of the LiipImagineBundle in the extension, check for the liip_imagine.cache.manager in a compiler pass. either way both should lead to removing the doctrine_phpcr.odm.subscriber.image_cache service?

@lsmith77

lsmith77 Mar 19, 2013

Member
diff --git a/DependencyInjection/DoctrinePHPCRExtension.php b/DependencyInjection/DoctrinePHPCRExtension.php
index 21c0b15..16b188e 100644
--- a/DependencyInjection/DoctrinePHPCRExtension.php
+++ b/DependencyInjection/DoctrinePHPCRExtension.php
@@ -261,6 +261,10 @@ class DoctrinePHPCRExtension extends AbstractDoctrineExtension
         $container->setParameter('doctrine_phpcr.odm.subscriber.image_cache.filter', $filter);
         $container->setParameter('doctrine_phpcr.odm.subscriber.image_cache.all_filters', $filters);
         $this->loader->load('odm_image.xml');
+        $bundles = $container->getParameter('kernel.bundles');
+        if (!isset($bundles['LiipImagineBundle'])) {
+            $container->removeDefinition('doctrine_phpcr.odm.subscriber.image_cache');
+        }

         $documentManagers = array();
         foreach ($config['document_managers'] as $name => $documentManager) {
@lsmith77

lsmith77 Mar 19, 2013

Member

committed this as a quick fix a94e1e9

@dbu

dbu Mar 19, 2013

Member

indeed you are right, sorry. i think this can also be the not-so-quick fix. or we could check the $filter variable to only remove if that was intended. then the user gets an error if he sets a filter but does not have imagine. shall i change to that?

- {% endif %}
+ {% block phpcr_odm_image_widget_preview %}
+ {% if form.vars.data and imagine_filter %}
+ <img src="{{ form.vars.data | imagine_filter( imagine_filter ) }}" alt="" />
@stof

stof Mar 19, 2013

Member

This does not remove the dependency AFAIK as filters are validated at compile time, not at runtime

@dbu

dbu Mar 19, 2013

Member

oh, that would be bad. is there a way to avoid the problem? or do we need to provide different twig files for the two cases?

@stof

stof Mar 20, 2013

Member

AFAIK, you need a different template

@dbu dbu referenced this pull request in symfony-cmf/media-bundle Jun 7, 2013

Closed

adding media interfaces and a download controller draft #2

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