template lookup should search controller inheritance chain

+1 I tried this on 2.1 and it works as expected. It's a very useful feature for people using inherited controllers.

The plugin handles partials as well.

I'll work on a patch now.

Running into some issues with actionmailer tests. Which reminds me, should I implement inheritable templates for actionmailer too?

In the name of consistency, it seems to make sense, but on the other hand, I feel like it'll never be used. So, for now, I'm skipping it, but if there's interest, post here.

I shouldn't have to touch ActionView, since it only affects the controller's default template. Maybe it's missing a hook?

I'm starting to think this would be better off as a plugin. However, it is sucky that you have to override the pick partial stuff. We should fix this so your plugin can hook in easier.

http://github.com/kch/inheritabl...

Template finding is quite spaghettied into the various rails components. Controllers find action templates one way, then views find partials another, and actionmailer works differently too.

Maybe this is something that could be abstracted to a template finder module?

As for the hooks, we used to have them. If you check out the tags in my plugin repository, you can see that for previous versions of rails I had to overwrite less internal stuff. Was never an ideal situation though.

On having to patch actionview, if you just patch actionview the way I did, and have it ask the controller for the template, then the plugin wouldn't need to orverride _pick_partial_template.

You don't need to call find_inherited_template, you could simply call default_template_name, and the plugin could override it to use find_inherited_template, as it does now.

However, all this feels pretty flaky. I'd prefer to have the whole thing committed to the core. Seems to make sense that templates would be inherited.

I'll try to work on a more robust patch.

Yet another attempt to get this feature through. The attached patch based on current 2-3-stable.

Oh no, not 3.0... I am desperately lost in that code ;)

Guys, quick doublecheck. Am I supposed to make 3.0 version at this point or the whole idea has to be first reviewed by Yehuda Katz?

I just talked to Yehuda and he's asking for a patch on master. He says this should be implemented in the Resolver, which currently doesn't have any way to specify fallbacks. (The "context" slot needs to take an array)

I am sure this will help you, artemave. Please do ask anything here about this. Thanks!

Rizwan, I'm finishing up the patch at the moment. However the change affects AbstractController::Rendering rather than Resolver. I'll look closely though into this alternative and possibly come up with two different patches.

Thanks for your response, it is good to know there is after all life on Mars.

You might wanna buckle yourself up to write this up since Yehuda is all set to release Rails 3 beta soon. Good Luck!

Well, this is roughly it.

The patch applies cleanly. But artemave, you might want to do it in Resolver as Yehuda had said. Anyways, will pass it on to Yehuda and let's see what he says.

Yehuda still wants it to be written in Resolver. Can you do that?

I am struggling to figure out how. Resolver has got no awareness of controller at the moment. Unless we start passing it to view_paths elements. Does that sound like a plan? Or is there something I am missing here?

This is Yehuda had in mind:

There's a slot for "context" right now, which is the controller directory to look under. I think that would have to take an Array of contexts. The problem with doing it in render is that it'll only work in some cases but not in others. People will be wondering why a layout name from a parent controller doesn't come in.

He also wonders whether it might good to leave this until Rails 3.1.

artemave, I hope this helps. Thanks!

Rizwan,

I've been waiting for 3 months for any response. If it wasn't for that, this feature would have been done long time ago. So I think it would be fair for you guys to wait few days for me to finish.

Sorry you had to wait that long. Get your patch ready, I will ask Yehuda to
apply it. Thanks!

When you talk about 'array of contexts' do you mean the array in FileSystemResolverWithFallback?

P.S.
Good point about layouts not being inherited (in my patch). Frankly, I didn't know that layouts get picked up much the same way as regular templates.

A quick note on layout inheritance:

Given controller A with layout "a", controller B < A, with an explicit call to layout false, B must not fallback layout "a", it must use no layout.

Just the kind of common bug that tends to crop up.

And thanks for taking this over from me.

No worries. This feature is a bit cursed though. Started here: http://dev.rubyonrails.org/ticket/5027 . Then failed to make it here: http://dev.rubyonrails.org/ticket/7076 . And got shot in this ticket. So, in the end, I just couldn't resist fighting the doom.

P.S.
Nice test case, thanks.

Speaking of layout inheritance. If implemented, it may change current behavior of existing applications. Some controllers may start falling back to parent layout instead of application layout (current behavior). Please let me know if this is a problem.

Here is the version WITHOUT layout inheritance.
Few notes:

• Template inheritance ended up in PathSet rather than Resolver. Since instance of Resolver is an element in view_paths, it can only "apply itself" to parent controller. But if parent controller has different elements in its view_paths this will be incorrect.

• MissingTemplate changed in order to comply with template inheritance. There were also some unused/out of place stuff there. As if it's been copypasted from 2.x code. I believe I cleaned up a bit.

• PathSet::find_template method seemed to not have been used anywhere. Hence, removed.

Version WITH layout inheritance may be produced quickly once the desired behavior is clarified (see my previous comment)

http://github.com/artemave/rails/tree/view_inheritance_resolver_3-0

Good job! Looks pretty nice so far. I will show this to Yehuda whenever (or wherever) I find him! :)

in_regards_to 'rails 3.0 beta release' do
  with :all_the_respect { artemave.says('Thanks for ignoring me').to($rails_team) }
end

Few changes:

• moved calculating parent controllers to AbstractController::Base
• rebased against latest master

http://github.com/artemave/rails/tree/view_inheritance_resolver_3-0

Can someone please reassign this ticket to someone who actually has time to deal with it? Seriously.

Maybe if carl types merge and yehuda types push, they can get it done this year. :P

Hey guys,

Not sure if you've been paying attention to master, but we've been making some changes to ActionController and ActionView, specifically around the Resolver. Currently, there are too many paths that result in template lookup, making patches like this far more complex than they should be.

We've been working on refactoring this part of the code with the express intention of making it trivial to implement this feature (without needing as much code as is in this patch). Part of that work involved reducing global state with the intention of removing AV's direct dependency on AC for much of what it does. Again, you can follow that progress on master.

In short, I haven't pulled this patch because it exposed a problem with our current architecture that Carl and I wanted to fix the right way.

As I've said before, this feature is desired. We're actively working on it (again, see master), and we're going to get it in properly.

Thanks for status update. It certainly cleared up the picture.

I am therefor waiting til you guys finish up fighting global state. And there we'll see what is next.

Please can anyone advise of the status of this issue? Did the relevant refactoring take place? It would be great to use this feature (or something very similar) in Rails 3.

Yet another go. http://github.com/artemave/rails/commit/a4de354d32a3bef0a5f14d5a4cc...

All tests are passing against current master.

+1 from Me. But it should be opt-in.

Good point, I believe this fixes it github.com/artemave/rails/commit/f9b87450b2d5bde0177502c5b5bba2761e3b96a4

Rebased against current master. Again. https://github.com/artemave/rails/tree/view_inheritance

Can someone from rails team respond/review, please?

I can't make it work on Rails 3.0.5, neither adding 'config.template_inheritance = true' to my application.rb file. Is it published on Rails 3.0, or will it be in 3.1?

As the milestone says, 3.1.