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. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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. On Dec 9, 2007 8:53 PM, Josh Peek <joshpeek@gmail.com> wrote:> > 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. > > >-- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On 12/9/07, Josh Peek <joshpeek@gmail.com> wrote:> 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.Nice work Josh! The beginning of a great and wondrous journey, I hope ;) Best, jeremy --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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:> On 12/9/07, Josh Peek <joshp...@gmail.com> wrote: > > > > > 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. > > Nice work Josh! The beginning of a great and wondrous journey, I hope ;) > > Best, > jeremy--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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 On Dec 10, 2007 3:06 AM, Josh Peek <joshpeek@gmail.com> wrote:> > 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. > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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! > - MislavI 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. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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 On Dec 10, 1:56 pm, Josh Peek <joshp...@gmail.com> wrote:> 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.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
There any thoughts on Nathans email? I''m a happy hamler and hate to see haml left out here. On Dec 11, 5:59 am, Nex3 <Nex...@gmail.com> wrote:> 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 > > On Dec 10, 1:56 pm, Josh Peek <joshp...@gmail.com> wrote: > > > 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.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
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. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---