fortrabbit / slimcontroller

Controller extensions for the Slim Framework, used by help.fortrabbit.com and blog.fortrabbit.com
MIT License
177 stars 17 forks source link

problem with template_suffix #37

Closed aurmil closed 9 years ago

aurmil commented 9 years ago

hi

it is not possible to set "controller.template_suffix" to empty string as this line can't be true with an empty string:

if ($renderTemplateSuffix = $app->config('controller.template_suffix')) {

I need to be able to set this config entry to empty string as I use Slim + Plates and my templates files are named "xyz.php" Plates already completes the ending ".php" so when adding your module I am forced to rename the templates files to "xyz.something.php" and set "controller.template_suffix" to "something" and I don't want my Plates config to be dependent on non-related modules presence

thanks for reading

yaworsw commented 9 years ago

Okay, so templates for slim controller are defaulted to the twig extension. I don't want to change that incase people are relying on that. You can override this though. If you extend SlimController and then define $renderTemplateSuffix in your subclass it will override the default.

<?php
// src/Application/Controller/AbstractController.php

namespace Application\Controller;

use SlimController\SlimController;

abstract class AbstractController extends SlimController
{

    protected $renderTemplateSuffix = '';

}
<?php
// src/Application/Controller/IndexController.php

namespace Application\Controller;

class IndexController extends AbstractController
{

    public function indexAction()
    {
        $this->render('index', array(

        ));
    }

}

Please let me know if you need any more help. I've got a 1/2 finished example slim controller app which overrides SlimController to (potentially) provide app specific customization.

https://github.com/yaworsw/example-slimcontroller-app/tree/master/src/TodoApp/Controller

aurmil commented 9 years ago

thanks for your answering

I totally agree, actual behavior shoud not be changed

but wouldn't it be possible to change the line 75 so empty values would be accepted?

to be able to do

'controller.template_suffix' => '',

actually, doing so does not accept empty string and used "twig" suffix

PS: could be possible to add a check on is_null if null => not set => twig if empty string => intentionaly set => use this value

yaworsw commented 9 years ago

Okay so I think you're right about how template_suffix should work. I created a new branch branch-0.5.0 to do development for v0.5.0. It includes changes to template_suffix. You can use these changes by specifying 398fb56d4c86d1766dd896421d3519060d67516a as your slim controller version in composer.json.

aurmil commented 9 years ago

I'm not sure to understand what you did in your commit by setting your attribute default value to empty string instead of "twig", ddidn't you break the behavior you said you didn't want to change? and IMO, problem comes from "__construct()", where "$this->renderTemplateSuffix" can't be set to empty string, not from "render()" for me,

if ($renderTemplateSuffix = $app->config('controller.template_suffix')) {

should look like the next "if" with "is_null":

if (!is_null($renderTemplateSuffix = $app->config('controller.template_suffix'))) {
yaworsw commented 9 years ago

You're right. I think threw that together a bit too quickly.

I think e79e666ac474212c1a28fc4304e0fb2acf0964b0 is better.

aurmil commented 9 years ago

seems to work as expected with this commit :)