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 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.
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
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: Parked at Loopia
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
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).
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:
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.
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.
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.
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).
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.
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.
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.
