template lookup should search controller inheritance chain
Reported by Caio Chassot | September 1st, 2008 @ 07:23 AM | in 2.x
(Josh, I briefly mentioned this to you via email.)
Here's the gist of the feature:
Say you have:
DocumentsController < ApplicationController ArticlesController < DocumentsController
and a view file at:
app/views/documents/list.html.erb
If you request articles/list, and app/views/articles/list.html.erb doesn't exist, rails should render app/views/documents/list.html.erb for you.
Here's my plugin that implements this behavior:
http://github.com/kch/inheritabl...
I'm opening this case to consider making this behavior default in rails.
If you would please experiment with the plugin and let me know if the core is interested, I'd be happy to convert the plugin into a patch.
Comments and changes to this ticket
-
Ahmed Adam September 4th, 2008 @ 05:53 AM
+1 I tried this on 2.1 and it works as expected. It's a very useful feature for people using inherited controllers.
-
Joshua Peek September 7th, 2008 @ 03:59 PM
- → Milestone changed from “2.x” to “2.2”
- → State changed from “new” to “open”
You can you use git-format-patch to make me a diff file, please.
I'm also curious how this applies to partial rendering. I ran into this problem a few times.
# admin/posts/index.html.erb render :partial => "post" # posts/_post.html.erb Hello -
Caio Chassot September 8th, 2008 @ 03:13 AM
The plugin handles partials as well.
I'll work on a patch now.
-
Caio Chassot September 8th, 2008 @ 04:11 AM
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.
-
Caio Chassot September 8th, 2008 @ 04:20 AM
- → Tag changed from “actionpack edge enhancement” to “actionpack edge enhancement patch”
Here's a patch.
Only handling actionpack, no actionmailer.
Passes all tests, no new tests added.
I think such a feature would deserve at least a test for the controller action and partial template lookup. Would appreciate some pointers on where it's best to add them.
-
Joshua Peek September 8th, 2008 @ 03:23 PM
I shouldn't have to touch ActionView, since it only affects the controller's default template. Maybe it's missing a hook?
-
Joshua Peek September 8th, 2008 @ 03:46 PM
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.
-
Caio Chassot September 8th, 2008 @ 06:24 PM
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.
-
Caio Chassot September 8th, 2008 @ 06:26 PM
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.
-
Joshua Peek September 8th, 2008 @ 11:25 PM
- → Milestone changed from “2.2” to “2.x”
:) I totally agree. If you could refactor some of that finder code i'd be more convinced to let it into core. I'm just a bit worried about adding the logic to default_template_name (where it should be) and to pick_template (where it shouldn't). I could see myself auditing pick_template sometime in the future and wondering why this code is there.
-
Joshua Peek September 8th, 2008 @ 11:26 PM
- → State changed from “open” to “wontfix”
Temporary closing the ticket. Can you please email me (directly) when you come up with a kick ass solution. Thanks for looking into this.
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Source available from github
The Git repository resides at http://github.com/rails
Check out the current development trunk (Edge Rails) with:
git clone git://github.com/rails/rails.git
Creating or reviewing a patch
See the contributor guide.
Creating a feature request
Please don't. If you want a new feature in Rails, you'll have to pull up your sleeves and get busy yourself. Or convince someone else to do it. See the contributor guide on how to get going. But posting them here is just going to lead to ticket root.
Creating a bug report
When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.
Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.
Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too".
