Skip to content
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

Removing the $this Context From Templates #9

Open
ghost opened this issue May 22, 2015 · 29 comments
Open

Removing the $this Context From Templates #9

ghost opened this issue May 22, 2015 · 29 comments
Labels

Comments

@ghost
Copy link

ghost commented May 22, 2015

Hi Giuseppe,

This isn't an issue, just a question.

I have an existing framework that uses the output buffer to load up and render templates, and I'd like to replace that with Foil, but I can't seem to get my existing variables to render because they don't use the $this context.

For instance in my controller I set all my variables into the $data array:

$data['products'] = $products;
$data['menu']     = $this->theme->controller('widget/header_menu');

Then push the $data variable to the view class (almost exactly the way you do in Foil.)

return $this->theme->view('content/header', $data);

In my View class, the data is extracted with the file, into the buffer, then sent to the user.

if (is_readable($this->file)):
    extract($this->data);
    ob_start();
    require $this->file;
    $output = ob_get_contents();
    ob_end_clean();

    return $output;
endif;

So my templates simply reference the name of the variable without the $this context:

<?php foreach($products as $product): ?>
     // do something with $product
<?php endforeach; ?>

<?= $menu; ?>

etc.

So what I'm asking I guess, is there any way to push my data to a Foil instance so that I don't have to edit all the templates.

I have over 500 templates, and my client isn't going to wait to have all those templates re-written.

Thanks for your help, great job on Foil, I hope it'll work for me.

--Vince

@gmazzap
Copy link
Contributor

gmazzap commented May 22, 2015

Hi Vince,

first of all thank for your interest in Foil.

A similar question have been already asked, in #4 you can find a discussion about the usage of $this.

However, I understand the problem of existing templates. At the moment the fastest way to do it, it's to write a class that extends Foil\Template\Template.

In there, you should theorically to override just one method, collect().

The problem is that that method is private, so you need to override also render().

In short your class should be something like:

class VinceTemplate extends \Foil\Template\Template {

    // just copied and pasted form Foil code
    public function render(array $data = [])
    {
        $this->api()->fire('f.template.prerender', $this);
        $this->setData(array_merge($this->data(), $data));
        $output = $this->collect($this->path());
        while ($this->layoutPath()) {
            $layout = $this->layoutPath();
            $this->setData($this->buildContext(
                $this->layout_data[$layout]['data'],
                $this->layout_data[$layout]['only']
            ));
            $this->layout = null;
            $this->api()->fire('f.template.renderlayout', $layout, $this);
            $output = $this->collect($layout);
        }
        $this->api()->fire('f.template.rendered', $this);
        return $output;
    }

    private function collect($path)
    {
        ob_start();
        $data = $this->data(); // <------------ NOTICE THIS
        extract($data);        // <------------ NOTICE THIS
        require $path;
        $this->last_buffer = $this->buffer;
        $this->buffer = trim(ob_get_clean());
        return $this->buffer;
    }
}

After that, you need to tell Foil to use your template class. If I was better, I'd made this task easier.

Feel free to open an issue to make the usage of custom templates easier.

Btw, it is doable.

The easiest way is to write a custom TemplateFactory class.

As you can see, default factory already allows to pass a custom template class, but actually is not so easy to do.

Your custom factory class should be something like:

class VinceFactory extends \Foil\Template\Factory
{
   public function factory($path, $class_name = null)
   {
       // be sure to use correct class with namespace
       return parent::factory( $path, 'VinceTemplate' );
   }
}

finally, when you use instantiate Foil, do something like this:

$engine = Foil\engine([
  'folders' => ['path/to/templates']
]);

// do this immediately after having created the engine instance
$container = Foil\foil();
$container['template.factory'] = new VinceFactory(
    $container['template.templates'],
    $containe['section.sections'],
    $containe['api']
);
// ends of customizations

$some_data = [
    'foo' => 'Foo',
    'bar' => 'Bar'
];

echo $engine->render('template-name', $some_data);

Please Note

that while writing this comment I found a bug in Foil that prevent this customization it to work.

I released v0.4.3 that fix it.

Please be sure to update before implementing.

...as Gist

In this Gist you can find the downloadable code.

It is my will to make usage of custom template classes easier, until that, you can use this approach, if you want... and, in case, tell me if it work :)

@ghost
Copy link
Author

ghost commented May 22, 2015

Hi Giuseppe,

Thanks so much for your help. I apologize for not seeing the #4 issue, I didn't check the closed ones.

If you don't mind I'll leave this open until I test so I can post back my results.

Looking at your gist, is there no way to get rid of using the buffer? That's the whole point for me in switching to a template system.

Thanks again, and I'll report back on my results.

@gmazzap
Copy link
Contributor

gmazzap commented May 22, 2015

is there no way to get rid of using the buffer?

Buffer is something needed to handle template inheritance, that is a core part of Foil, TBH I wrote foil because I was not happy on how other non compiled engines handle inheritance.

No time now, but as soon I can I'll try to find a solution that may work in your case.

If you don't mind I'll leave this open until I test so I can post back my results.

Absolutely. I think I'll leave this open until I improve that part of Foil, so other people can easily read it. This Q has been already asked 2 times, so it is a FAQ and it needs an answer somewhere.

I'll report back on my results.

Thank you a lot for that.

@ghost
Copy link
Author

ghost commented May 22, 2015

Works like a charm!

I noticed that the only thing I actually needed to make this work was to add a single line of code to the collect method in the Template class.

private function collect($path)
    {
        ob_start();
        extract($this->data()); // added this line
        require $path;
        $this->last_buffer = $this->buffer;
        $this->buffer = trim(ob_get_clean());

        return $this->buffer;
    }

I think I may just fork the repo and set the url in my composer.json.

Ideally if you wanted to make Foil more flexible you can add these methods and update the documentation.

public function extractRender(array $data = [])
{
    $this->api()->fire('f.template.prerender', $this);
    $this->setData(array_merge($this->data(), $data));
    $output = $this->extractCollect($this->path());
    while ($this->layoutPath()) {
        $layout = $this->layoutPath();
        $this->setData($this->buildContext(
            $this->layout_data[$layout]['data'],
            $this->layout_data[$layout]['only']
        ));
        $this->layout = null;
        // listener for this event makes sections work in output mode
        $this->api()->fire('f.template.renderlayout', $layout, $this);
        $output = $this->extractCollect($layout);
    }

    $this->api()->fire('f.template.rendered', $this);

    return $output;
}

private function extractCollect($path)
{
    ob_start();
    extract($this->data());
    require $path;
    $this->last_buffer = $this->buffer;
    $this->buffer = trim(ob_get_clean());

    return $this->buffer;
}

That would be great for those that need to escape that $this context.

If you like I can fork the repo and submit a pull request with those updates, I know you're busy.

Thanks again mate.

--Vince

@gmazzap
Copy link
Contributor

gmazzap commented May 22, 2015

Thanks @19peaches

I think I have an idea to make this thing much more simple. Let me found some time to test it and I'll post here my efforts.

@gmazzap
Copy link
Contributor

gmazzap commented May 22, 2015

@19peaches That was muuuch more easy I thought :)

The only thing required is to create a proxy template.

This proxy template, let's assume is saved in a file named proxy-template.php, have to contain this code:

$template = Foil\engine()->find($this->template);

if (is_file($template)) {
    extract($this->a('template_data', []));
    require $template;
}

After that, you can just do:

$engine = Foil\engine([
    'folders' => [
         'my'    => '/path/your/templates/',
         'proxy' => '/path/to/proxy/template/folder/'
    ]
]);

$some_data = [
    'foo' => 'Foo',
    'bar' => 'Bar'
];

$template_name = 'example-template';

echo $engine->render(
    'proxy::proxy-template',
    [
        'template'      => "my::{$template_name}",
        'template_data' => $some_data
    ]
);

That's all. The only thing needed was create a template that act as a mediator between Foil and your current templates.

If you test it, let me know.

@ghost
Copy link
Author

ghost commented May 22, 2015

Yeah I'm not doing all that to avoid changing a single line of code.

I've forked the repo as you know and I've set my composer.json to use my repo.

Feel free to point folks who need this to my repo.

Thanks for all your help.

--Vince

@gmazzap
Copy link
Contributor

gmazzap commented May 25, 2015

Hi Vince,

with version 0.5 of Foil, usage of custom template classes is much easier.

Now you can extend Template class and override the collect method that is protected and no more private:

namespace Vince;

class ExtractTemplate extends Foil\Template\Template
{
   protected function collect($path)
   {
      ob_start();
      extract($this->data());
      require $path;
      $this->last_buffer = $this->buffer;
      $this->buffer = trim(ob_get_clean());

      return $this->buffer;
  }
}

and then tell Foil to use your custom template class:

$engine = Foil\engine([
  'folders'        => ['path/to/templates'],
  'template_class' => 'Vince\\ExtractTemplate'
]);

see "Custom Template Classes" in doc page.

I know that this requires a class to be added to your project, but that will allow you to use main Foil package, and take advantage of any improvements and bug fixes that may be done in this repo.

Anyway, I'll keep this issue opened so anyone interested in render extracted variables in templates can easily find a solution here.

@ghost
Copy link
Author

ghost commented May 25, 2015

Hey Giuseppe,

Great job on this. Unfortunately this still doesn't work for me. Sensio Insights will still throw the require/include flag if I use your new method.

Using the Foil Template class within the vendor directory keeps the flag out of my project which was the main point of looking for a template engine for my project.

I created a free standing clone of Foil so I could run Foil through Sensio Insights.

Here's the report for my foil repo.

I've already corrected a couple issues, but you may want to create an account there if you don't have one and run through these issues.

Thanks.

-Vince

@gmazzap
Copy link
Contributor

gmazzap commented May 25, 2015

Hi Vince, and thanks for your work.

I don't use Sensio Insight because it doesn't really works :)

I use phpStorm tools, Code Sniffer and PHP Mess Detector to find problems in my code.

Regarding the problem with require it is raised by Sensio Insight because it thinks that require is used to require a class file. Well, it isn't.

It is used to require a template file. That's a perfectly legit use, and actually does not exists a different way to do that.

If you look at Symfony Templating Component, it does the same thing in the exact same way.

Do you know that Symfony is developed by SensioLab, just like Insight?

phpStorm analitics tools recognize that require usage as a problem too, because being a dynamic require it is not possible for the IDE understand which file code tries to require.

This is the reason for the /** @noinspection PhpIncludeInspection */ annotation: it says to phpStorm "don't worry about this require, I know what I'm doing". But Sensio Insight don't understand that annotation.

No automatic tool can ever make code analysis like a human being.

If your target is a gold metal from Insight, than takes Insight issues as rules, if your target is having good code that works, take them as suggestions, and don't give up the right to disagree.

In fact, how demonstrated above (and below, too), even SensioLab code raise Insight issues.

Below, one by one, there are all the reported issues with an explaination why they are not (all but one) real issues.


.DS_Store is user-specific and should not appear in a project .gitignore. Consider adding it to the user global .gitignore instead.

Working on a fork, project .gitignore is the global .gitignore. If I remove that entry, collaborators that uses OSX may be forced to delete .DS_Store from their forked repo folder to avoid commiting it. So, why I should remove that line making life of collaborators possibly harder considering that it hurts in no way? Moreover, a .DS_Store is not very different from Thumbs.db in Windows, that is also in .gitignore. Why does Sensio Insight complain about .DS_Store but not about Thumbs.db?


This args attribute is declared but never used. You should remove it.

A quick look at code shows that variable is used just 7 lines after declaration. Sensio Insight seems to not see that for who knows which reason.


This escapeObject method is declared but never used. You should remove it.

If you look at code you'll see that the method escapeObject() is called via $this->$method where $method can be "escapeString", "escapeArray" or "escapeObject".

I added @see phpDoc entries to make clear that these methods are actually used. phpStorm understands that, Sensio Insight doesn't.


This container local variable is declared but never used. You should remove it.

Actually $container variable is used to store in it some providers. (example). In fact, variable is used because container object is actually modified.

The code I used is the suggested way to use Pimple service providers as can be seen in the Pimple documentation itself.

So Sensio Insight doesn't recognize an usage case that is suggested in the official documentation of a product that belongs to its same company...


Adding "@" before filemtime($path) prevents warning and errors during this function execution from being displayed. If you need to do that, you should probably implement a better way to qualify and recover from errors, using Exceptions.

"preventing warning and errors from being displayed" is the intended behavior, there. I don't want that warning and errors are displayed in rendered templates. For any reason. Once this is not something that can stop application to work, is perfectly and widely accepted use @ to prefix file operations, as long as the option that function does not work properly is take into account. And in my code is. In fact, just 3 lines after I check that function has returned a non-empty value.

Btw, in next release I'm planning a minor reformatting and maybe I'll make use of try / catch block there, and even if behavior will be exactly the same Insight should not complain anymore.


The class Traversable is declared but never used. You should remove the use statement.

This one is the only reported issue that makes sense. I solved this in last commit.

@ghost
Copy link
Author

ghost commented May 25, 2015

Totally agreed, I have a few items in my project that I had to comment and ignore on Insight. I'm not sure I'd go so far as to say it doesn't work, but it certainly doesn't understand anything dynamic or take into consideration that you may need to require files that aren't classes.

I'll go ahead and just use your library and your Extract class solution and just do an ignore on Insight.

Cheers mate.

@ghost
Copy link
Author

ghost commented May 26, 2015

That method worked like a charm Giuseppe.

Thanks again.

@gmazzap
Copy link
Contributor

gmazzap commented May 26, 2015

Glad to hear that, @19peaches

@ghost
Copy link
Author

ghost commented Jul 15, 2015

Giuseppe,

I'm trying to run Foil as a service on a Pimple container with Laravel style facades.

My service sets up foil like so:

$app['foil'] = function ($app) {
    return \Foil\engine([
        'ext'            => 'tpl',
        'folders'        => [Theme::getPath() . 'view'],
        'template_class' => 'MyApp\Base\View'
    ]);
};

Can you detail how to set up Foil as a Pimple service?

Thanks.

@gmazzap
Copy link
Contributor

gmazzap commented Jul 16, 2015

If $app is an instance of Pimple, like I guess, that's perfectly fine and avoid to deal with internal Foil objects.

A more structured alternative, as of Foil >= v0.6, and using Pimple service provider would be:

namespace MyApp;

use Pimple\Container;
use Pimple\ServiceProviderInterface;
use Foil\Foil;
use Foil\Extensions\Uri;

class FoilServiceProvider implements ServiceProviderInterface
{

    public function register(Container $pimple) {

         $pimple['foil.options'] = [
               'ext'            => 'tpl',
               'template_class' => 'MyApp\Base\View'
               // if "Theme" is a class that is inside container, is better to use that
               // class to get path, e.g. $pimple['theme']->getPath()
               'folders'        => [Theme::getPath() . 'view'],
          ];

          $pimple['foil.boostrap'] = function(Container $pimple)
          {
                return Foil::boot($pimple['foil.options']);
          };

          // an example on how Foil extesions can be placed in the mix
          $pimple['foil.uri.options'] = [
                'host'   => 'example.com',
                'scheme' => 'https',
                'urls'   => [
                     'admin' => '/path/to/admin',
                     'blog'  => '/path/to/blog'
                ];
          ];
          $pimple['foil.uri'] = function(Container $pimple)
          {
                return new Uri();
          };

          $pimple['foil'] = function(Container $pimple)
          {
                $engine = $pimple['foil.boostrap']->engine();

                $engine->loadExtension($pimple['foil.uri'], $pimple['foil.uri.options']);

                return $engine;
          };
    } 

}

Then use the provider with your container:

$app->register(new FoilServiceProvider());

Now $app['foil'] can be used to access Foil engine.

@ghost
Copy link
Author

ghost commented Jul 16, 2015

Fatal error: Class 'Foil\Foil' not found

@ghost
Copy link
Author

ghost commented Jul 16, 2015

Yeah I couldn't get that to work.

This seems to work great:

namespace MyApp\Services\Base;

use Pimple\Container;
use Pimple\ServiceProviderInterface;

class ViewService implements ServiceProviderInterface {

    public function register(Container $app) {
        $options = [
            'ext'            => 'tpl',
            'folders'        => [$app['theme']->getPath() . 'view'],
            'template_class' => 'MyApp\Base\View'
        ];

        $app['foil'] = function (Container $app) use ($options) {
            return \Foil\engine($options);
        };
    }
}

Let me know if there are any drawbacks to using it this way.

Thanks for your help.
-Vince

@gmazzap
Copy link
Contributor

gmazzap commented Jul 16, 2015

No, there is no drawback on instantianting Foil like that.

By the way, I think you are using a version older than 0.6. As I said in previous comment, Foil\Foil was introduced in 0.6.0 version. (Current is 0.6.2).

Note that version introduced some backward incompatible changes, so it you want to upgrade be sure to read release notes before.

@ghost
Copy link
Author

ghost commented Jul 16, 2015

"foil/foil": "~0.6",

Is this not the right one?

@ghost
Copy link
Author

ghost commented Jul 16, 2015

This is what composer gave me:

Updating dependencies (including require-dev)
  - Updating foil/foil (0.5.1 => dev-master 863a54b)
    Checking out 863a54b9bc3a964f670a12baa2826d2d25c2936a

@gmazzap
Copy link
Contributor

gmazzap commented Jul 16, 2015

@19peaches yes, "foil/foil": "~0.6" is fine.

Thanks to your comment I just discovered an issue with Packagist, that did not updated the source when I moved Foil to an organization.

Can you please run composer update again? Thanks a lot.

@ghost
Copy link
Author

ghost commented Jul 16, 2015

Excellent.

Just a note:

$pimple['foil.uri'] = function(Container $pimple)
{
    return Uri();
};

Throws class not found error.

$pimple['foil.uri'] = function(Container $pimple)
{
    return new Uri();
};

Works.

Thanks Giuseppe.

@gmazzap
Copy link
Contributor

gmazzap commented Jul 17, 2015

@19peaches off course, was a typo.

@thinsoldier
Copy link

So my templates simply reference the name of the variable without the $this context:

Is there no way to simply get a collection of all the variables that were made available to the view and convert that to an array and then run extract( $arrayOfThingsFrom$This ) on the first line of the actual view files so that the variable name you want without the need for a $this-> context exists?

@gmazzap
Copy link
Contributor

gmazzap commented Sep 4, 2016

@thinsoldier the data that was passed to render can be obtained using $this->data() inside templates.

Point is that I didn't want to use extract and still I don't want to use it. However, I provided a way to make it possible for people who want to use other Foil features use context without $this.

Note that last versions of Foil include the "alias" feature that allows referencing variable with another "container" that is not $this, for example is possible to use $T as alias and inside template just use $T->var_name so the difference with $var_name is just 3 characters. A "price" that I am very happy to pay considering the benefit that comes with it.

@thinsoldier
Copy link

thinsoldier commented Sep 5, 2016

@Giuseppe-Mazzapica Is there a specific advantage of using the alias feature over using $foil = $this; manually?

And if the only variables in my view are the ones coming from $this->data() what is the draback of using extract( $this->data ) ?

@gmazzap
Copy link
Contributor

gmazzap commented Sep 5, 2016

@thinsoldier

Is there a specific advantage of using the alias feature over using $foil = $this; manually?

If you do that, you have to do that in all the templates, and all the partials... Using alias feature Foil will do that for you. Internally what Foild does is ${$alias} = $this; before render any template and partial.

And if the only variables in my view are the ones coming from $this->data() what is the draback of using extract( $this->data )?

The problem with extract( $this->data ) is... extract.

That function is evil because doesn't allow to track down where variables are defined. So, for example, if you get a notice like variable $foo not defined it's hard to find where that var should be defined, so debug becomes much harder. Speaking of variables not defined, using $this->var_name allows to 1) don't throw notices when a variable is not defined, escpecially on production environments and 2) automatic variable escaping.

@thinsoldier
Copy link

@Giuseppe-Mazzapica I'm sorry if I seem like a troll. I'm not, really!
What if I wanted a notice to be reported when an undefined variable is used?
My production environment is set to log all warnings, notices and errors to a file and never display them in output.

@gmazzap
Copy link
Contributor

gmazzap commented Sep 6, 2016

@thinsoldier Since version 0.3 there's the option strict_variables that allow to throw notices when undefined variabled are used or alternatively to throw exceptions. So you have more control ;) See documentation (scroll down to bottom).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants