I haven''t had a chance to work on fixing multiple controller view paths recently. My original patch attempt was: http://dev.rubyonrails.org/ticket/8582 It was rejected due to the fact that it was fixing the symptom more than the problem. However, I think it''s critical that this problem get fixed, otherwise the whole concept of view_paths is severely neutered and doesn''t even live up to the use case advertised in the announcement on the official blog (ie. productization). The way view_paths currently work is only suitable as a sort of half-measure towards providing Engines functionality. That''s a worthy goal, but I think the ability to modify view_paths per-request is at least as important if not more so. I will try to find time to work on a better patch, but I want to make sure this gets into Rails 2.0 because we don''t want to support a broken interface later. How close are we to an RC? -- Gabe da Silveira http://darwinweb.net --~--~---------~--~----~------------~-------~--~----~ 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 3-okt-2007, at 0:39, Gabe da Silveira wrote:> I haven''t had a chance to work on fixing multiple controller view > paths recently. My original patch attempt was: > > http://dev.rubyonrails.org/ticket/8582I pluginized an implementation that turned out to work pretty well. I''d say if we formalize absolute paths for the "render" method and totally delegate the lookup to a specific object that can be overridden the problem is solved twofold. I also heard from the maintainer of Typo that he is planning on implementing Themer there. I didn''t here any feedback from you though.... -- 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 -~----------~----~----~----~------~----~------~--~---
On 10/3/07, Julian Tarkhanov <listbox@julik.nl> wrote:> > > I pluginized an implementation that turned out to work pretty well. > I''d say if we formalize absolute paths for the "render" method > and totally delegate the lookup to a specific object that can be > overridden the problem is solved twofold. I also heard from the > maintainer of Typo that he is planning on implementing Themer there. > > I didn''t here any feedback from you though....I know, sorry. I''ve been swamped and had to put all this on the back burner. However with Rails 2.0 coming there is a certain urgency. --~--~---------~--~----~------------~-------~--~----~ 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 10/3/07, Julian Tarkhanov <listbox@julik.nl> wrote:> > > On 3-okt-2007, at 0:39, Gabe da Silveira wrote: > > > I haven''t had a chance to work on fixing multiple controller view > > paths recently. My original patch attempt was: > > > > http://dev.rubyonrails.org/ticket/8582 > > I pluginized an implementation that turned out to work pretty well. > I''d say if we formalize absolute paths for the "render" method > and totally delegate the lookup to a specific object that can be > overridden the problem is solved twofold. I also heard from the > maintainer of Typo that he is planning on implementing Themer there.I just saw that patch in trac last night. I think it looks interesting, and I''ll be taking a closer look at it later this week when I get a chance... -- Rick Olson http://lighthouseapp.com 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 3-okt-2007, at 22:19, Rick Olson wrote:> > I just saw that patch in trac last night. I think it looks > interesting, and I''ll be taking a closer look at it later this week > when I get a chance...if you are speaking about the themer approach there is one requirement: the render method should accept absolute paths and the directory from which they are considered absolute should be public, known and hardcoded with respect to RAILS_ROOT. The amount of junk I had to implement to feed AV with relative paths is just embarassing IMO :-) -- 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 -~----------~----~----~----~------~----~------~--~---
On 10/2/07, Gabe da Silveira <gabriel.d@gmail.com> wrote:> I haven''t had a chance to work on fixing multiple controller view paths > recently. My original patch attempt was: > > http://dev.rubyonrails.org/ticket/8582 > > It was rejected due to the fact that it was fixing the symptom more than the > problem. > > However, I think it''s critical that this problem get fixed, otherwise the > whole concept of view_paths is severely neutered and doesn''t even live up to > the use case advertised in the announcement on the official blog (ie. > productization). The way view_paths currently work is only suitable as a > sort of half-measure towards providing Engines functionality. That''s a > worthy goal, but I think the ability to modify view_paths per-request is at > least as important if not more so. > > I will try to find time to work on a better patch, but I want to make sure > this gets into Rails 2.0 because we don''t want to support a broken interface > later. How close are we to an RC?I think the dup/freeze stuff was added so you didn''t inadvertently cause memleaks by constantly adding paths per-request. Here''s a patch I''m playing around with: http://pastie.caboo.se/103921.txt It basically delegates the #view_paths instance method to @template so you can easily access. I also removed all duping and freezing, since they won''t be modified in most rails apps anyway. Any thoughts on this? I used this in a test controller to test per-request paths: def index self.view_paths = [self.class.view_paths[params[:path].to_i]] end I think it actually makes more sense if ActionView::Base gets a duped view_paths though. It establishes that the controller class view_paths are the default, while the controller instances''s view_paths only affect that request: def index self.view_paths.unshift self.class.view_paths[params[:path].to_i] end -- Rick Olson http://lighthouseapp.com 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 5-okt-2007, at 1:28, Rick Olson wrote:> > I think it actually makes more sense if ActionView::Base gets a duped > view_paths though. It establishes that the controller class > view_paths are the default, while the controller instances''s > view_paths only affect that request:Not to be rude, but this solution seems half-cooked (or half-baked) to me. It''s still a hack. Why don''t we abolish the view_paths altogether in favor of some solution that is a) encapsulated b) officially overridable c) does not muck with inheritable class_vars That is: 1 template path per controller point (what we had in rails 1.2.3), the rest via a lookup object that responds to partial_path, action_path, layout_path and mail_path. This way you escape the mess and give people enough flexibility. The way in which the lookup object can be implemented is left totally to the developer of the application. This way you also remove a substantial layer of complexity from _both_ ActionView and ActionController, you avoid GC problems that you mentioned and you don''t make "workaround" solutions where no workarounds are needed (this is should not be so convoluted!) The only thing you need is: 1 ivar per request (the template lookup object) and _proper_ path support for ActionView (published way to feed it absolute paths). render :action => ''/foo/bar/baz'' # we know that "/" means RAILS_ROOT/ app/views point, no exception - you can compute the path to anything _from there_ render :action => ''foo/bar/baz'' # we use the template finding implemented in Rails 1.2.3 A solution like that seems also in line to the latest Rails improvements (implementing plugin hooks and extracting instead of making more layers upon layers of indirection). -- 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 -~----------~----~----~----~------~----~------~--~---
> A solution like that seems also in line to the latest Rails > improvements (implementing plugin hooks and extracting instead of > making more layers upon layers of indirection).We can push something like that into ActionView once we''ve branched for 2.0, along the lines of charlie''s suggestions: http://cfis.savagexi.com/articles/2007/09/16/making-rails-better-actionview-needs-a-makeover There''s a *lot* of room to improve actionview, but it can wait for 2.1 when we can do it properly. -- 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 -~----------~----~----~----~------~----~------~--~---
On 5-okt-2007, at 23:01, Michael Koziarski wrote:> We can push something like that into ActionView once we''ve branched > for 2.0, along the lines of charlie''s suggestions: > > http://cfis.savagexi.com/articles/2007/09/16/making-rails-better- > actionview-needs-a-makeover > > There''s a *lot* of room to improve actionview, but it can wait for > 2.1 when we can do it properly.Well, I''ve got a radical proposal then. Why don''t we _abolish_ the multiple view paths until 2.1 hits the streets? I mean, it''s edge functionality, people using it must have known what they are doing. John Long doesn''t seem to be providing any feedback. And it''s misimplemented AFAIK. If you ask me I consider these multiple view paths more breakage than an medicine, it''s an underpowered, crippled solution which, thus far, has broken all of my apps and has complicated the situation instead of giving a cure. So let''s put it so: I''m all for a refactoring run (and I will have some time to put into that shortly), but this abomination has to go for 2.0. Implementing a custom theming strategy is also much simpler with a single template_root. -- 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 -~----------~----~----~----~------~----~------~--~---
On 5-okt-2007, at 23:06, Julian Tarkhanov wrote:> If you ask me I consider these multiple view paths more breakage than > an medicine,Sorry - to elaborate - if the class-based view path array is released with 2.0 then you will be bound to support it like.... forever -- 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 -~----------~----~----~----~------~----~------~--~---
On 10/5/07, Julian Tarkhanov <listbox@julik.nl> wrote:> > > On 5-okt-2007, at 23:06, Julian Tarkhanov wrote: > > > If you ask me I consider these multiple view paths more breakage than > > an medicine, > > Sorry - to elaborate - if the class-based view path array is released > with 2.0 then you will be bound to > support it like.... forever >I don''t think so. View path array is very useful in the current state, and if the API gets better and view paths start to have more features after Rails 2.0 I don''t see how that will break apps for people upgrading from 2.0 . --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Sorry - to elaborate - if the class-based view path array is released > with 2.0 then you will be bound to > support it like.... foreverWe''d be obliged to support the public interfaces of rails for 2.0.x. Not crazy monkeypatching plugins. In the event we get a better implementation we just have to have a documented upgrade path for 2.1 -- 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 -~----------~----~----~----~------~----~------~--~---
On 5-okt-2007, at 23:19, Mislav Marohnić wrote:> View path array is very useful in the current stateI wonder in which ways it is superior to the template_root that was there before? It does not properly work with inheritance (you need the same workarounds as with before_ and after_filter, because managing an inherited array is nasty), it is classlevel, it does not couple to ActionMailer, it does not solve the problems in ActionView. Basically the only thing it _does_ permit is making a kind-of-working overlay stack for template lookups (purposedly for plugins as I''ve read on the changesets), once per class definition in the code. For the rest of us it makes life much more difficult. Seems like I''m the minority on the issue anyway. I would love to know what do the Engines folks and John Long think about it (John especially because he made the patch in the first place). -- 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 -~----------~----~----~----~------~----~------~--~---
On 6-okt-2007, at 0:05, Michael Koziarski wrote:> >> Sorry - to elaborate - if the class-based view path array is released >> with 2.0 then you will be bound to >> support it like.... forever > > We''d be obliged to support the public interfaces of rails for 2.0.x. > Not crazy monkeypatching plugins.The public interface for a shared, inherited, class-contained array is very hairy, so it''s about the same. -- 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 -~----------~----~----~----~------~----~------~--~---
On 06/10/2007, Julian Tarkhanov <listbox@julik.nl> wrote:> > > On 5-okt-2007, at 23:19, Mislav Marohnić wrote: > > > View path array is very useful in the current state > > I wonder in which ways it is superior to the template_root that was > there before? > It does not properly work with inheritance (you need the same > workarounds as with before_ and after_filter, > because managing an inherited array is nasty), it is classlevel, it > does not couple to ActionMailer, it does not solve the problems > in ActionView. Basically the only thing it _does_ permit is making a > kind-of-working overlay stack for template lookups (purposedly for > plugins as I've read on the changesets), once per class definition in > the code. For the rest of us it makes life much more difficult.Depends what you mean by difficult. In order to get Typo's themes working with Rails 2.0, I ended up doing: @@view_paths[self.class.name] [path_to_theme] + @@view_paths["ActionController::Base"] in a before filter and it worked like a charm. Obviously, I expect it'll get broken in subsequent Rails releases, but it turned out to be a great deal easier to do it this way than it did to work out how to use the themer plugin, which works fine until you try and work out some way of not having to pass in an explicit :themer argument to every call to render - I kept ending up in an infinite loop, which is no fun at all. --~--~---------~--~----~------------~-------~--~----~ 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 6-okt-2007, at 5:16, Piers Cawley wrote:> I kept ending up in an infinite > loop, which is no fun at all.It''s OT, but it''s a matter of overriding render in a controller that has to be themed by default. def render(*args) return super(*args) if (args.any? && args.last.is_a?(Hash) && args.last[:inline]) (args << {}) if args.empty? || !args[-1].is_a?(Hash) args[-1][:themer] = @themer if @themer super(*args) end Nevermind. I get that I''m a minority here, so I''ll step aside and see what the more enlightened folks will come up with :-) will need hacking anyways. -- 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 -~----------~----~----~----~------~----~------~--~---
On 10/6/07, Julian Tarkhanov <listbox@julik.nl> wrote:> Seems like I''m the minority on the issue anyway. I would love to know > what do the Engines folks > and John Long think about it (John especially because he made the > patch in the first place).As far as I''m concerned, per-request view paths aren''t something I''ve craved; really all I wanted was a simple way to have a $LOAD_PATH-style set of directories which were sequentially inspected for a given template, and I believe that is now possible, certainly in http://dev.rubyonrails.org/changeset/7034... unless I''m mistaken? -- * 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 -~----------~----~----~----~------~----~------~--~---
On 10/6/07, James Adam <james.adam@gmail.com> wrote:> > > As far as I''m concerned, per-request view paths aren''t something I''ve > craved; really all I wanted was a simple way to have a > $LOAD_PATH-style set of directories which were sequentially inspected > for a given template, and I believe that is now possible, certainly in > http://dev.rubyonrails.org/changeset/7034... unless I''m mistaken? >Absolutely yes it''s been possible for a while. The original announcement of multiple controller view paths on the official blog mentioned productization of apps, which definitely requires per-request view paths unless you want each custom site to have it''s own memory footprint. --~--~---------~--~----~------------~-------~--~----~ 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 06/10/2007, Julian Tarkhanov <listbox@julik.nl> wrote:> > > On 6-okt-2007, at 5:16, Piers Cawley wrote: > > > I kept ending up in an infinite > > loop, which is no fun at all. > > It''s OT, but it''s a matter of overriding render in a controller that > has to be themed by default. > > def render(*args) > return super(*args) if (args.any? && args.last.is_a?(Hash) && > args.last[:inline]) > (args << {}) if args.empty? || !args[-1].is_a?(Hash) > args[-1][:themer] = @themer if @themer > super(*args) > end > > Nevermind. I get that I''m a minority here, so I''ll step aside and see > what the more enlightened folks will come up with :-) > will need hacking anyways.I''m pretty sure that that''s what I tried. The problem is that, some way down the call chain, we end up calling render again without the themer in the argument list, so it bounces up to the top level render, which detects the absence of a themer and therefore inserts one, and then it''s off round the loop again. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Depends what you mean by difficult. In order to get Typo''s themes > working with Rails 2.0, I ended up doing: > > @@view_paths[self.class.name] > [path_to_theme] + > @@view_paths["ActionController::Base"] > > in a before filter and it worked like a charm.In my proposed patch I removed the class variables in favor of class instance variables. Now you''d do: self.view_paths = [path_to_theme] + superclass.view_paths Though I wonder if append/prepend should do that? In my patch: def prepend_view_path(path) view_paths.unshift(*path) end How about: def prepend_view_path(path) self.view_paths = superclass.view_paths.dup if @view_paths.nil? view_paths.unshift(*path) end class ActionController::Base self.view_paths = [default] end class FooController prepend_view_path custom_foo_path end -- Rick Olson http://lighthouseapp.com 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 -~----------~----~----~----~------~----~------~--~---