Refactored to inject helper rather than extending it. #68
Conversation
| @@ -46,6 +46,15 @@ public function __construct(SecurityContextInterface $publishWorkflowChecker = n | |||
| } | |||
| } | |||
|
|
|||
| protected function getDm() | |||
There was a problem hiding this comment.
@lsmith77 I think this is actually the right way to do it anyway - I believe we should also add setManagerName and that this method should return the document manager from the registry depending on the value of $managerName ??
There was a problem hiding this comment.
in this case it might however make sense to assign the manager to a local variable for those methods that use the manager multiple times.
|
Fixed up service.xml and added basic DI test for twig extension. |
|
the helper service should be tagged as a php template helper |
|
i am +1 on this change, looks a lot cleaner. do the tests all pass? seems travis is not looking at PR of the CoreBundle. @wouterj this is changing the refactoring you did, do you agree this makes sense? |
|
@dbu well, I requested to do also the change here :) It turns out that my refactoring results in an error for PHP 5.0.0 til 5.3.8, because both interfaces (template helper interface and twig extension interface) have a method So, this is the only way to do it... |
| @@ -4,8 +4,15 @@ | |||
|
|
|||
| use Symfony\Cmf\Bundle\CoreBundle\Templating\Helper\CmfHelper; | |||
|
|
|||
| class CmfExtension extends CmfHelper implements \Twig_ExtensionInterface | |||
| class CmfExtension implements \Twig_ExtensionInterface | |||
There was a problem hiding this comment.
please remove the implement and extend Twig_Extension and remove all method (except getName) after line // from \Twig_Extension (line 41)
|
@dantleech can you do the same fixes as in the SonataBlockBundle here too? otherwise i think its ready to merge. would be good to have that in the next release. |
|
Updated. |
Refactored to inject helper rather than extending it.
|
thanks! |
|
thank you @dantleech for the quick fix. Shall I submit a fix for the block bundle? |
No description provided.