Following up on: http://wiseheartdesign.com/2006/12/6/rails-needs-something-better-than-engines/ I''ve created a patch that deprecates ActionController::Base.template_root in favor of ActionController::Base.view_paths (which will allow you to specify multiple paths from which to load views): http://dev.rubyonrails.org/attachment/ticket/2754/view_paths.diff ActionMailer and Rails::Config still need to be updated to reflect this change, but I wanted to submit it for feedback before going any further. Any thoughts? -- John Long http://wiseheartdesign.com/ --~--~---------~--~----~------------~-------~--~----~ 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 1/3/07, John Long <johnwlong2000@gmail.com> wrote:> > > Following up on: > > > http://wiseheartdesign.com/2006/12/6/rails-needs-something-better-than-engines/ > > I''ve created a patch that deprecates > ActionController::Base.template_root in favor of > ActionController::Base.view_paths (which will allow you to specify > multiple paths from which to load views) > Any thoughts?I liked that article very much. I think Rails 2.0 definitely has to be more flexible about this (judging from the number of aforementioned hacks), and that this patch is a good start - not too much shock, and not too little improvement. I''m hoping for a sweet point where Rails and Engines would meet in stable harmony. Dynamic and modular web apps aren''t at all dynamic or modular if they can''t interact or share functionality. -M --~--~---------~--~----~------------~-------~--~----~ 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 1/3/07, John Long <johnwlong2000@gmail.com> wrote:> > Following up on: > > http://wiseheartdesign.com/2006/12/6/rails-needs-something-better-than-engines/ > > I''ve created a patch that deprecates > ActionController::Base.template_root in favor of > ActionController::Base.view_paths (which will allow you to specify > multiple paths from which to load views): > > http://dev.rubyonrails.org/attachment/ticket/2754/view_paths.diff > > ActionMailer and Rails::Config still need to be updated to reflect > this change, but I wanted to submit it for feedback before going any > further. > > Any thoughts? >This looks like a good idea to me. We''ve had to hack around the template_path stuff in some of our apps, and I know this would help us out and also the many plugins that hook into it. - Rob --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> http://dev.rubyonrails.org/attachment/ticket/2754/view_paths.diff > > ActionMailer and Rails::Config still need to be updated to reflect > this change, but I wanted to submit it for feedback before going any > further. > > Any thoughts?The 1.2 boat has sailed, but perhaps there''s room for something like this in 2.0. A few things to consider though: * Why''d you change deprecation.rb? * Should .template_root= override or supplement previously supplied values to .view_paths* Should we just expose the array, or provide methods to ''add_view_path'' * Does this play right with people''s expectations around inheritance. etc. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Michael Koziarski wrote:> The 1.2 boat has sailed, but perhaps there''s room for something like > this in 2.0. A few things to consider though: > > * Why''d you change deprecation.rb?Because you can''t presently deprecate setter methods (methods with ending in "="). A setter method can''t take a block.> * Should .template_root= override or supplement previously supplied > values to .view_pathsThe patch presently does this: def template_root=(path) view_paths.unshift(path) end def template_root view_paths.first end But it could also be defined like this: def template_root=(path) self.view_paths = [path] end def template_root view_paths.first end> * Should we just expose the array, or provide methods to ''add_view_path''I really like just exposing the array. It harmonizes well with Config#additional_load_paths and Config#controller_paths.> * Does this play right with people''s expectations around inheritance.Inheritance is tricky. I thought about creating a custom class InheritedArray that would allow changes to the parent class''s array to filter down, but decided it would be overkill. It presently is defined like this: def view_paths if paths = @@view_paths[name] paths else if superclass.respond_to?(:view_paths) superclass.view_paths.dup.freeze else @@view_paths[name] = [] end end end Which causes the subclass to return a frozen version of the superclass''s array if it doesn''t have it''s own array defined. This is to prevent modification of the superclass''s array when doing: subclass.view_paths << ''additional/path'' # raises an error Instead you need to set it directly: subclass.view_paths += [ ''additional/path'' ] Somewhat messy, but it seemed the nicer alternative. -- John Long http://wiseheartdesign.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Michael Koziarski wrote: > > The 1.2 boat has sailed, but perhaps there''s room for something like > > this in 2.0. A few things to consider though:Michael and others, any other thoughts on this? I''m eager to see it applied. -- John Long http://wiseheartdesign.com --~--~---------~--~----~------------~-------~--~----~ 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 1/5/07, John W. Long <johnwlong2000@gmail.com> wrote:> > > Michael Koziarski wrote: > > > The 1.2 boat has sailed, but perhaps there''s room for something like > > > this in 2.0. A few things to consider though: > > Michael and others, any other thoughts on this? I''m eager to see it > applied.Personally, I''m just not in favor of this. I definitely see the usefulness in something like Radiant, Mephisto, anything using Engines, etc. But, it seems like a waste of time for every other app. What''s cool about this patch is how you''ve got the important view logic extracted into smaller methods such as #lookup_template_base_path_for and #find_base_path_for. Is there any way that we can just extract that and allow this view_paths plugin to work without any hardcore jiggery-pokery? -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 1/5/07, Rick Olson <technoweenie@gmail.com> wrote:> What''s cool about this patch is how you''ve got the important view > logic extracted into smaller methods such as > #lookup_template_base_path_for and #find_base_path_for. Is there any > way that we can just extract that and allow this view_paths plugin to > work without any hardcore jiggery-pokery?The main locus for template location in ActionView seems to be the full_template_path method, the internals of which John''s path has replaced. It''s also the only significant part of ActionView that the engines plugin touches, where it treats any app/views directories in plugins as a kind of view_load_path. Having multiple view paths gets most complicated, however, when working with ActionMailer''s use of ActionView and multipart emails, because of the way it automatically searches for various templates based on the part type (specifcally in the ActionMailer::Base#create! method, and the ActionMailer::Base#initialize_template_class method - monkeypatchers beware, ActionMailer overrides this internally itself). Without a ''proper'' implementation, any web application that wants to enable view loading from multiple locations with reasonable flexibility has to hack a bit. That said, I have no problems with this not being in core either. For ''base'' applications (like Radiant or Mephisto) that might be extended by users with plugins that aren''t appropriate for general rails applications, it shouldn''t be difficult to include the necessary overriding code as an internal lib, right? -- * J * ~ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Rick Olson wrote:> Personally, I''m just not in favor of this. I definitely see the > usefulness in something like Radiant, Mephisto, anything using > Engines, etc. But, it seems like a waste of time for every other app.When you say a "waste of time" you mean you have performance concerns about it or something else? It would be simple to write a common plugin to make this work, but I thought plugins that depend on other plugins are discouraged. For me, multiple view paths is just a small step towards seeing Rails support better modularization. -- John Long http://wiseheartdesign.com --~--~---------~--~----~------------~-------~--~----~ 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 1/5/07, John W. Long <johnwlong2000@gmail.com> wrote:> > > Rick Olson wrote: > > Personally, I''m just not in favor of this. I definitely see the > > usefulness in something like Radiant, Mephisto, anything using > > Engines, etc. But, it seems like a waste of time for every other app. > > When you say a "waste of time" you mean you have performance concerns > about it or something else? It would be simple to write a common plugin > to make this work, but I thought plugins that depend on other plugins > are discouraged. > > For me, multiple view paths is just a small step towards seeing Rails > support better modularization.I agree with John, this allows for better modularization and prevents plugins from depending on plugins. I don''t personally see why this would perform worse than the original version when it is configured with only one path (the default). I would certainly support this since I have to do quite a bit of work to get this type of functionality for MasterView and this would clean things up and provide a standard way to hook in. The risk of not going with this approach is that each and every plugin that needs to hook into this functionality might do things differently causing problems, when it really feels like something that should be in the rails core. Even if there is a plugin that encapsulates it, every plugin developer needs to be aware of that plugin and all of them use it. Where as if it is just part of the rails core api, then it is a no brainer that plugin developers try to use standard rails functionality before going to extensions. So I think making it simpler to hook in (as long as it doesn''t make things too complicated or cause big performance hits, which I don''t believe this does) is a good thing. Jeff --~--~---------~--~----~------------~-------~--~----~ 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 1/5/07, John W. Long <johnwlong2000@gmail.com> wrote:> > Rick Olson wrote: > > Personally, I''m just not in favor of this. I definitely see the > > usefulness in something like Radiant, Mephisto, anything using > > Engines, etc. But, it seems like a waste of time for every other app. > > When you say a "waste of time" you mean you have performance concerns > about it or something else? It would be simple to write a common plugin > to make this work, but I thought plugins that depend on other plugins > are discouraged. > > For me, multiple view paths is just a small step towards seeing Rails > support better modularization. >+1 -- Rails needs more support easy modularization like this, especially when it would be totally transparent to people who don''t need it. We really don''t want to continue down the road of each plugin/engine/etc hacking stuff in its own way... - Rob --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> > When you say a "waste of time" you mean you have performance concerns > > about it or something else? It would be simple to write a common plugin > > to make this work, but I thought plugins that depend on other plugins > > are discouraged.Well, you can now order plugins whichever way you like in environment.rb. Plus, having it as a plugin now means folks can try out the view_paths stuff without any commitment from core. If it''s really not a burden, maybe the rest of the plugin can be added to core. Rails 2.0 is still a ways off. -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
http://dev.rubyonrails.org/changeset/6120 committed. have fun (though, not with trac apparently). -- Rick Olson http://weblog.techno-weenie.net http://mephistoblog.com --~--~---------~--~----~------------~-------~--~----~ 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 Feb 4, 2007, at 10:03 PM, Rick Olson wrote:> http://dev.rubyonrails.org/changeset/6120 > > committed. have fun (though, not with trac apparently).I just so totally dig this thx John and Rick -- Julian ''Julik'' Tarkhanov please send all personal mail to me at julik.nl --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---