nervetattoo / li3_twig

Fork of the Lithium Twig integration
5 stars 7 forks source link

Twig templates are now a type of their own. #5

Closed marcghorayeb closed 11 years ago

marcghorayeb commented 12 years ago

Here's a cleaner pull request. This enables us to use layout extensions (instead of the lithium two step rendering). Tests need to be updated but the library works correctly :)

nervetattoo commented 12 years ago

There are some issues here:

  1. API changes. Having to set render-type on a per-controller basis is breaking
  2. You've upgraded Twig itself inside the pull request, thats an unrelated change and should be a single commit of it own.
  3. Changes for how helpers work: Why?
  4. The config for the library should all be defaulted and handled in bootstrap so its only overridden when something different is needed
  5. You've hardcoded type to html, should use {:type} like before.

I have not yet tested any of the code, and yes, the tests cant break if thats what you're implying. In fact, I want more tests as better coverage is needed — not less, or tests breaking ;)

But it looks a lot better than the last PR, thanks :)

marcghorayeb commented 12 years ago

1) I found it to be the only 'simple' way to make it not overwrite the default PHP renderer. This is a non-issue, I always have a base controller from which all the other controllers inherit, and you have that line only once to make it a default. Controllers can still override it down the process. 2) My mistake. 3) The new method makes use of integrated Twig dynamic functions, you don't need to override the template class anymore. The old method is still available, but Twig doesn't like helpers that don't return anything (scripts or style with inline option to false for example), so you need to override them anyways if you don't want ugly exceptions. 4) Ok I'll change that. 5) If I use type, that means the files need to be called view.htmlTwig instead of view.html.twig. This breaks the whole concept of types as you can still have view.ajax.twig for example. Unless you handle ajax and html version in one twig file which can become quite a pain for big views/projects.

I'm available on irc if you want to discuss more on the matter :)