Generate a different controller when Twig is not installed #7
Conversation
|
|
||
| private function isTwigInstalled() | ||
| { | ||
| return class_exists('Symfony\Bundle\TwigBundle\TwigBundle') |
There was a problem hiding this comment.
you can use the class constant here
There was a problem hiding this comment.
You mean this? class_exists(TwigBundle::class) and the use ... import or class_exists(Symfony\Bundle\TwigBundle\TwigBundle::class) ? I don't like it because the editor shows a "not found class" error.
There was a problem hiding this comment.
But as I said, the current version works as expected:
but the other proposal makes your editor display an error (it's annoying and confusing):
There was a problem hiding this comment.
Well, that's we do in the core too. So why do we want to change that here? Indeed, the error is not so nice, but that's related to the IDE/editor. And the end user will probably not look into this class anyway.
There was a problem hiding this comment.
If this is what Symfony does, there's nothing more to discuss 😄 Consistency is the most important thing.
|
|
||
| private function isTwigInstalled() | ||
| { | ||
| return class_exists(Symfony\Bundle\TwigBundle\TwigBundle::class); |
There was a problem hiding this comment.
You should add a use Symfony\Bundle\TwigBundle\TwigBundle statement and then use TwigBundle::class here.
| public function index() | ||
| { | ||
| // replace this line with your own code! | ||
| return $this->render('@Maker/demoPage.html.twig', [ 'path' => str_replace($this->getParameter('kernel.project_dir').'/', '', __FILE__) ]); |
There was a problem hiding this comment.
I would use demo.html.twig as a template name instead. @Maker won't work anyway and we don't recommend using @App.
There was a problem hiding this comment.
In a separate PR I'm going to propose to remove this code and the demo template entirely. We don't need it in a generator bundle (it's useful only after creating a new project, but we already have that covered).
|
I'm having some tech issues updating this PR ... so I've created another PR: #8. |
This fixes #6.