Those of us that need to use custom inflections with RESTful route path helpers know that routes get loaded before custom inflections get set in the environment ( ). However, we’ve had a simple workaround for this by forcing the routes to reload in the environment.rb, after we define our inflections… Inflector.inflections do inflect.irregular ‘cave’, ‘caves’ end ActionController::Routing::Routes.reload and life was good again… until Rails 2.0PR Revision [7369] ( ) specifically prevents the reloading of the routes if they have already been loaded. So, life sucks again. The patch provides a new reload! method (with an !) that forces the reload, so updating the environment.rb to ActionController::Routing::Routes.reload! gets around it… but anybody else trying to upgrade to 2.0 that is using custom inflections with RESTful route path helpers is going to need to hunt down this thread and make that change themselves, or figure it out on their own (after previously needing to figure out the need to reload routes in the first place). [7369] doesn’t include any tests showing that it fixes anything (I thought that was one of the new rules!? ), nor do the comments with the revision give any hints to it’s purpose or ticket it closes. I’m not seeing the benefit and it’s definitely going to introduce unnecessary headaches for a fraction of the Rails community. So, I’m curious what this change was for, and if it’s actually helping anybody. Doesn’t “reload”, by it’s very name imply that the user knows that the routes are already loaded, but they want to “REload” them anyway? ‘reload!’ seems redundant to me. Before [7369] it was just ‘load!’ and it’s alias ‘reload’. Instead of changing the functionality of ‘reload’ and adding a ‘reload!’ method, how about a separate method ( unfortunately ‘load’ is already used by Ruby ) that provides the new “only load if not already loaded” functionality and let ‘reload’ continue to be an alias of ‘load!’ Ultimately I’d like to see the need for the first workaround completely go away, but that may be easier said than done, and I’m not sure that ticket #6829 (linked above) provides the best solution.
[7369] doesn't include any tests showing that it fixes anything (I thought that was one of the new rules!?
), nor do the comments with the revision give any hints to it's purpose or ticket it closes. I'm not seeing the benefit and it's definitely going to introduce unnecessary headaches for a fraction of the Rails community. So, I'm curious what this change was for, and if it's actually helping anybody.
It's a siginificant performance improvement in development mode, I've noticed it and I'm hugely happy with it :). reload isn't a documented part of the API so I don't think it's that big a deal. reload could be named something like reload_unless_routes_dot_rb_hasnt_been_changed, but I don't think that's justified.
A one character change for those painting outside the lines doesn't seem like it's too much to worry about. Besides, couldn't you just put the inflections *above* the initializer block?
Michael Koziarski wrote:
[7369] doesn't include any tests showing that it fixes anything (I thought that was one of the new rules!? ;-) ), nor do the comments with the revision give any hints to it's purpose or ticket it closes. I'm not seeing the benefit and it's definitely going to introduce unnecessary headaches for a fraction of the Rails community. So, I'm curious what this change was for, and if it's actually helping anybody.It's a siginificant performance improvement in development mode, I've noticed it and I'm hugely happy with it :).
That’s good to hear.
reload isn't a documented part of the API so I don't think it's that big a deal. reload could be named something like reload_unless_routes_dot_rb_hasnt_been_changed, but I don't think that's justified.
No, I agree.
A one character change for those painting outside the lines doesn't seem like it's too much to worry about.
I don’t know that I would label those who write their application in
languages other than English as “painting outside the lines” Nor
simply needing things like a waves_controller, and wanting to be able
to call wave_path(1), instead of “wafe_path(1)”
Besides, couldn't you just put the inflections *above* the initializer block?
That would be a fantastic solution if it worked, except it results in…
“uninitialized constant Inflector”
Any solution that managed to setup the custom inflections before the routes would make me very happy (and I’d be very willing to work on the patch once pointed in the right direction). Other ideas?
I think we need a real fix for the issue. Requiring inflections to be done about the initializer block is no better than requiring some internal method call. Either way, the developer will have to analyze the issue then find the work-around.
I would support some kind of mechanism where the inflector itself is responsible for reloading routes if it is modified. It’s a little ugly, but if we throw a test in there it makes a huge potential headache go away.
I think we need a real fix for the issue. Requiring inflections to be done about the initializer block is no better than requiring some internal method call. Either way, the developer will have to analyze the issue then find the work-around.
Yeah, the issue does need to be fixed.
I would support some kind of mechanism where the inflector itself is responsible for reloading routes if it is modified. It's a little ugly, but if we throw a test in there it makes a huge potential headache go away.
That's a pretty ugly solution, but if it's all that can be done then so be it.
I have taken a shot: http://dev.rubyonrails.org/ticket/9815
Nice work, applied.
Jon and Gabe, does it fix things for you?
