Refactoring ActionView Template Handlers

[Josh Peek]

I've been doing some deep digging around ActionView lately, and
thought I'd clean some stuff up while I'm down there.

My first problem is ActionPack is really tightly coupled with ERB. My
first instinct was to take advantage of the template handlers. I
thought I could just refactor ERB, RJS, and Builder into nice
TemplateHandlers and bind them with register_template_handler. Well,
those template handlers are kind of second class because ActionView
will not compile them into speedy templates.

I started working on it and I found a ton of other "dumb" things
dropping out as well, like all this TEMPLATE_HANDLER_PREFERENCES non
sense. My initial patch does alot of cleanup, but I'm asking for some
feedback before I continue further.

http://dev.rubyonrails.org/ticket/10437

I think alot of the other template compile logic that is in
ActiveView::Base private methods could be move over to the
TemplateHandlers as well.

[Pratik]

Looks great so far. And as you're already at it, probably you could
look a bit into thread safety as well. That'd mainly be not to modify
any class variables ( global behavior ) from any methods which are
called while rendering, I suppose.

--
Cheers!
- Pratik
http://m.onkey.org

[Jeremy Kemper]

Nice work Josh! The beginning of a great and wondrous journey, I hope ;)

Best,
jeremy

[chuyeow]

I know it's already been committed, but just wanted to say that that
is awesome work Josh!

On Dec 10, 5:46 am, "Jeremy Kemper" <jer...@bitsweat.net> wrote:

[Josh Peek]

On Dec 9, 3:13 pm, Pratik <pratikn...@gmail.com> wrote:
> Looks great so far. And as you're already at it, probably you could
> look a bit into thread safety as well. That'd mainly be not to modify
> any class variables ( global behavior ) from any methods which are
> called while rendering, I suppose.

Yeah, that's good thing to watch out for.

Thanks, I've been wanting to get around to this since this post
(http://groups.google.com/group/rubyonrails-core/browse_thread/thread/
d1a1f92c94d7112e).

BTW, I'm not done. There is a few more things I had in mind. First
clear all traces of "erb", "rjs", "builder", etc exceptions in
ActionPack. Searching for those terms should only turn up results in
there respective template handler. Also clean up my TODO notes that
got committed :).

I'm also going try to move the compile template/caching stuff to
another module so "base.rb" reads a bit cleaner.

Please post any other ideas yea got.

[Mislav Marohnić]

Awesome stuff, Josh.

The side effect is that you broke Haml, but I'm sure Haml guys will fix it tonight as they are aware of the change.

Here is to better ActionView!
- Mislav

[Josh Peek]

On Dec 10, 2:25 pm, "Mislav Marohnić" <mislav.maroh...@gmail.com>
wrote:

> Awesome stuff, Josh.
>
> The side effect is that you broke Haml, but I'm sure Haml guys will fix it
> tonight as they are aware of the change.
>
> Here is to better ActionView!
> - Mislav

I looked at the HAML source before even starting this, and they don't
even use the registered template handlers. They just override the
render method. I think this will give them a better "official" way to
patch in.

[Nex3]

As a Haml guy, I guess I should chip in. This patch is a great step
forward in terms of template delegation, which was always pretty
awkward (hence the monkeypatching to get caching working).

However, it expects the template handlers to give a compiled Ruby
string that's then rolled into a method by ActionView. Haml does
produce such a string, but it's only dealt with internally, and it
would be awkward to expose it directly for various reasons. What I'm
using in the current monkeypatched code, and what I'd like to continue
making use of, is Haml::Engine#def_method, which takes an object, a
method name, and a list of local variables, and defines a method on
the object with the given name that renders the template with those
locals. It's pretty much identical to the work done by
ActionView::Base#create_template_source, except it doesn't create the
method name and it does evaluate the method definition.

If the source-generation/compilation stuff could be refactored out and
made overridable by the individual template handlers, that would be
ideal.

Otherwise, the main issue is that the Haml source has a non-standard
line offset. If this could be set on a per-handler basis, instead of
in ActionView::Base#compile_template, I could add a source accessor to
Haml::Engine. But I'd rather not.

- Nathan

[Spongy]

There any thoughts on Nathans email? I'm a happy hamler and hate to
see haml left out here.

[Josh Peek]

On Dec 10, 11:59 pm, Nex3 <Nex...@gmail.com> wrote:
> If the source-generation/compilation stuff could be refactored out and
> made overridable by the individual template handlers, that would be
> ideal.
>
> Otherwise, the main issue is that the Haml source has a non-standard
> line offset. If this could be set on a per-handler basis, instead of
> in ActionView::Base#compile_template, I could add a source accessor to
> Haml::Engine. But I'd rather not.

Yeah, I want really want to see what was need by other templates
(HAML) before I moved to much into the TemplateHandler class. I tried
to push all the template compiling stuff into the TemplateHandler
class, so like you said, you could just override what you needed (But
that failed on my first attempt). But please, give it a shot and see
what else you can push into the TemplateHandler.