New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support controllers out of bundles #293

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
10 participants
@elnur
Contributor

elnur commented Apr 5, 2014

Need this to support bundleless application code. Currently using my own bundle for that, but would be nice to have this in the bundle itself.

@elnur

This comment has been minimized.

Show comment
Hide comment
@elnur

elnur Apr 10, 2014

Contributor

Ping @fabpot. Any thoughts on this?

Contributor

elnur commented Apr 10, 2014

Ping @fabpot. Any thoughts on this?

@elnur elnur referenced this pull request Apr 21, 2014

Merged

Fix a typo #298

@redstar504

This comment has been minimized.

Show comment
Hide comment
@redstar504

redstar504 commented Apr 23, 2014

👍

@drgomesp

This comment has been minimized.

Show comment
Hide comment
@drgomesp

drgomesp Apr 29, 2014

This would be awesome. 👍

drgomesp commented Apr 29, 2014

This would be awesome. 👍

@gnutix

This comment has been minimized.

Show comment
Hide comment
@gnutix

gnutix May 27, 2014

Big 👍 !

gnutix commented May 27, 2014

Big 👍 !

@drgomesp

This comment has been minimized.

Show comment
Hide comment
@drgomesp

drgomesp commented Jun 7, 2014

Ping @fabpot.

@hacfi

This comment has been minimized.

Show comment
Hide comment
@hacfi

hacfi Jun 14, 2014

👍 Would be quite useful!

hacfi commented Jun 14, 2014

👍 Would be quite useful!

Show outdated Hide outdated Templating/TemplateGuesser.php
@@ -96,6 +101,6 @@ protected function getBundleForClass($class)
$reflectionClass = $reflectionClass->getParentClass();
} while ($reflectionClass);
throw new \InvalidArgumentException(sprintf('The "%s" class does not belong to a registered bundle.', $class));
return null;

This comment has been minimized.

@rybakit

rybakit Jun 20, 2014

return is not needed here, see symfony/http-foundation@2ed84ed.

@rybakit
Show outdated Hide outdated Templating/TemplateGuesser.php
@@ -2,6 +2,7 @@
namespace Sensio\Bundle\FrameworkExtraBundle\Templating;
use Symfony\Component\HttpKernel\Bundle\BundleInterface;

This comment has been minimized.

@rybakit

rybakit Jun 20, 2014

It's not needed here, as it's only used in the @return phpdoc, which can be updated with FQCN.

@rybakit

rybakit Jun 20, 2014

It's not needed here, as it's only used in the @return phpdoc, which can be updated with FQCN.

This comment has been minimized.

@stof

stof Jun 20, 2014

Contributor

our coding standards are to use the short name in the phpdoc

@stof

stof Jun 20, 2014

Contributor

our coding standards are to use the short name in the phpdoc

@javiereguiluz

This comment has been minimized.

Show comment
Hide comment
@javiereguiluz

javiereguiluz Jul 8, 2014

Member

This would be a really nice feature! Thanks @elnur. Let's see if the Symfony Core Team (@fabpot, @stof, @romainneutron, @Tobion, @nicolas-grekas, @jakzal, etc.) agree with this proposal!

Member

javiereguiluz commented Jul 8, 2014

This would be a really nice feature! Thanks @elnur. Let's see if the Symfony Core Team (@fabpot, @stof, @romainneutron, @Tobion, @nicolas-grekas, @jakzal, etc.) agree with this proposal!

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Jul 9, 2014

Contributor

This looks good.

could you rebase your branch @elnur ? It conflicts with master

Contributor

stof commented Jul 9, 2014

This looks good.

could you rebase your branch @elnur ? It conflicts with master

@elnur

This comment has been minimized.

Show comment
Hide comment
@elnur

elnur Jul 9, 2014

Contributor

@stof, done.

Contributor

elnur commented Jul 9, 2014

@stof, done.

Show outdated Hide outdated Templating/TemplateGuesser.php
@@ -97,6 +101,6 @@ protected function getBundleForClass($class)
$reflectionClass = $reflectionClass->getParentClass();
} while ($reflectionClass);
throw new \InvalidArgumentException(sprintf('The "%s" class does not belong to a registered bundle.', $class));
return;
}

This comment has been minimized.

@fabpot

fabpot Jul 9, 2014

Member

Can you remove the return; as it is not needed?

@fabpot

fabpot Jul 9, 2014

Member

Can you remove the return; as it is not needed?

Show outdated Hide outdated Tests/Templating/Fixture/Controller/OutOfBundleController.php
@@ -0,0 +1,6 @@
<?php
namespace Sensio\Bundle\FrameworkExtraBundle\Tests\Templating\Fixture\Controller;

This comment has been minimized.

@fabpot

fabpot Jul 9, 2014

Member

Can you add a blank line after <?php?

@fabpot

fabpot Jul 9, 2014

Member

Can you add a blank line after <?php?

public function testGuessTemplateWithoutBundle()
{
$templateGuesser = new TemplateGuesser($this->kernel);
$templateReference = $templateGuesser->guessTemplateName(array(

This comment has been minimized.

@fabpot

fabpot Jul 9, 2014

Member

Can you put everything on one line?

@fabpot

fabpot Jul 9, 2014

Member

Can you put everything on one line?

This comment has been minimized.

@elnur

elnur Jul 9, 2014

Contributor

Are you sure about this one? It's done the same way in 3 other places in this test case. If I do what you suggest, it will be inconsistent.

@elnur

elnur Jul 9, 2014

Contributor

Are you sure about this one? It's done the same way in 3 other places in this test case. If I do what you suggest, it will be inconsistent.

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Jul 9, 2014

Member

Besides my small CS comments, 👍

Member

fabpot commented Jul 9, 2014

Besides my small CS comments, 👍

Show outdated Hide outdated Templating/TemplateGuesser.php
@@ -80,7 +85,6 @@ public function guessTemplateName($controller, Request $request, $engine = 'twig
*
* @param string $class A fully qualified controller class name
* @return Bundle $bundle A Bundle instance

This comment has been minimized.

@Tobion

Tobion Jul 9, 2014

Contributor

Bundle|null

@Tobion

Tobion Jul 9, 2014

Contributor

Bundle|null

This comment has been minimized.

@elnur

elnur Jul 9, 2014

Contributor

Are you sure I have to fix code that I haven't touched in this PR?

@elnur

elnur Jul 9, 2014

Contributor

Are you sure I have to fix code that I haven't touched in this PR?

This comment has been minimized.

@elnur

elnur Jul 9, 2014

Contributor

Never mind. My changes affects this.

@elnur

elnur Jul 9, 2014

Contributor

Never mind. My changes affects this.

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Jul 9, 2014

Contributor

Besides cs 👍

Contributor

Tobion commented Jul 9, 2014

Besides cs 👍

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Jul 10, 2014

Member

Thank you @elnur.

Member

fabpot commented Jul 10, 2014

Thank you @elnur.

@fabpot fabpot closed this in 1542921 Jul 10, 2014

@hacfi

This comment has been minimized.

Show comment
Hide comment
@hacfi

hacfi Jul 10, 2014

Thanks @elnur - will update right away :).

hacfi commented Jul 10, 2014

Thanks @elnur - will update right away :).

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