Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Karel Minařík October 22nd, 2008 @ 10:14 PM

  +1 on this one.

  This would be wonderful solution for persistent locale passing in URLs, as:

  
  def default_url_options(options={})
    { :locale => I18n.locale }
  end
  

  would pass locale set eg. in a before_filter to every route and to params[:locale].

  Karel

• 

  Pratik November 17th, 2008 @ 02:34 PM

  • Assigned user set to “Michael Koziarski”
• 

  Michael Koziarski November 17th, 2008 @ 02:36 PM

  • State changed from “new” to “resolved”

  This is actually fixed in 2.2 already.

  In fact I have that exact same snippet of code in my app :)

• You flagged this item as spam.
  

  Karel Minařík November 17th, 2008 @ 03:00 PM

  Hi Michael,

  thanks for the update!

  You got me thrilled :)

  I tried to use it, and the one major problem with restful routes still stands, at least in my test case.

  This declaration:

  @@ <%= book_path('cz', 1) %> @@

  is obviously fine, but:

  @@ <%= book_path(1) %> @@

  gives an error:

  @@ Exception while calling 'book_path(1)': #1, :controller=>"books", :action=>"show"}, expected: {:controller=>"books", :action=>"show"}, diff: {:locale=>1}> @@

  I have put the test app on Github, because frankly I don't know how to test/demonstrate it otherwise.

  http://github.com/karmi/test_def...

  Would you be so kind and have a look what I am doing wrong?

  Thanks!

  Karel

• 

  Michael Koziarski November 17th, 2008 @ 03:03 PM

  You can't use positional arguments with default_url_options sadly, you have to use

  book_path(:id=>2)

• You flagged this item as spam.
  

  Karel Minařík November 17th, 2008 @ 03:06 PM

  Ha! That does work!

  But it's kinda incovenient to use this syntax all over the place instead of "normal" book_path(1)...

  Do you think the positional arguments can be fixed or is it something too complex?

  --karmi

• 

  Michael Koziarski November 17th, 2008 @ 03:06 PM

  This is just a side-effect of how the positional arguments work, they fill in the arguments from left to right.

  so it's passing '2' for your locale. This has been like this for a while and is pretty much an inherent limitation unfortunately.

• You flagged this item as spam.
  

  Karel Minařík November 17th, 2008 @ 03:14 PM

  OK, thanks very much for this update!

  I think something like book_path(1) is something (at least) I can live with :)

  Updated the silly test app @ Github. Probably need some mention in the Guides/docrails.

  Thanks again!

  --karmi

• 

  Michael Koziarski November 17th, 2008 @ 03:16 PM

  It's also annoying as it breaks polymorphic_url sometimes, kinda frustrating. This would be something that could be fixed in another ticket if you wanted to help.

  But the positional arguments only have two options. Fill from right to left, or left to right.

  Using right to left would fix this problem, but would break people with values like locale at the right.

• 

  José Valim November 17th, 2008 @ 05:22 PM

  I attached a diff (not a patch yet) that allows us to do:

  map.user ':locale/user/:id'

  And then:

  I18n.locale = 'en-US' user_url(1) #=> http://host/en-US/user/1

  I18n.locale = 'pt-BR' user_url(1) #=> http://host/pt-BR/user/1

  I18n.locale = 'pt-BR' user_url('en-US', 1) #=> http://host/en-US/user/1

  It just adds some extra code to named routes helpers if those routes have a :locale as segment (very straightforward).

  Maybe it can be a start point to work in a solution that doesn't require us to write:

  user_url(:id => 1)

  Currently everything happens automagically, but we could use defaults key:

  map ':locale/user/:id', :defaults => { :locale => 'pt-BR' }

  So if it has a default value, it's not always required as positional argument. Finally, that wouldn't solve all our problems, so we could add proc handling to defaults:

  map ':locale/user/:id', :defaults => { :locale => proc{ I18n.locale } }

  Of course, just brainstorming. =)

  • routes_locale_wise.diff 1.4 KB
• 

  Michael Koziarski November 17th, 2008 @ 06:56 PM

  Right, this works for :locale, but not for anything else.

  We can't fix this with a special case that only suits the people with this one particular problem. after all, people can use Accept-Language or cookies for locales too :)

• You flagged this item as spam.
  

  Karel Minařík November 17th, 2008 @ 07:29 PM

  Right, this works for :locale, but not for anything else.

  Yes, that is the problem with Josés solution. :locale isn't something which should be hardcoded in Rails codebase.

  people can use Accept-Language or cookies for locales too

  No no no no no, please don't mention that again :)

  As extensively debated, cookies/sessions are the worst storage for locale. The resource ("page") should display the same content for everyone, no matter if (s)he has a cookie or not.

  (And Accept-Language is only slightly better. It may work for "application" type of app, eg. Lighthouse, but not for a content-oriented app, such as a magazine, website, etc.)

• 

  José Valim November 17th, 2008 @ 07:41 PM

  Exactly, my solution is hardcoded but I would gladly implement something.

  We just need to define a syntax, for example:

  map.user ':locale/user/:id', :optional => :locale

  Or:

  map.page '/page/:page/:id', :optional => :page

  Then we could get the :locale or :page from default_url_options.

  What do you think?

• You flagged this item as spam.
  

  iain January 10th, 2009 @ 01:34 AM

  • Tag changed from “2.2, l10n, routing, url_for” to “2.3, l10n, routing, url_for”

  I might have found the solution, tell me what you think: the keys provided by default_url_options are not filled by the options provided in the url_helper. Have a look at my patch, and tell me what you think.

  Sorry for it to take so long. Had to wait for this commit to be able to find the right eval ;)

  • default_url_options.patch 9.6 KB
• 

  José Valim January 10th, 2009 @ 08:29 AM

  It looks like it will brake some things because you are always removing the route segments. Example:

  
  class UsersController
    def default_url_options
      { :locale => 'en' }
    end
  
    def show
      # ...
      redirect_to user_url('en', 1)
    end
  

  Most application are doing like that right?

  But you are always removing the locale segment and then when you call zip in the route generation, 'en' will match the id segment and 1 will match nothing.

  A solution could be:

  
  if route_segments.size > args.size && respond_to?(:default_url_options) && default_url_options.respond_to?(:symbolize_keys)
    route_segments -= default_url_options.symbolize_keys.keys
  end
  

  Doing "route_segments.size > args.size" you will just remove the segment when not enough args are sent.

  Finally, I'm not sure if your implementation will work on views and helpers, because default_url_options method is not defined there. Or you will have to ask it for the controller or you will have to define default_url_options there.

• You flagged this item as spam.
  

  iain January 10th, 2009 @ 01:30 PM

  Most application are doing like that right?

  You wouldn't need to, because it is already specified in the default_url_options. It's a choice: either specify it in the default_url_options, or specify it namelessly in the user_url. Besides, you can still override it, using user_url(1, :locale => 'fr'), when you're changing locales for instance. Doing user_url('en', 1) isn't very DRY, when needing to specify the locale every time you call a URL. I doubt many applications will go through the trouble of changing their URLs everywhere, and I'm sure everyone wants to have to have clean method_calls.

  Doing "route_segments.size > args.size" you will just remove the segment when not enough args are sent.

  In Rails 2.3, there are almost always more route_segments than args, because the format is a route_segment. formatted_user_url is depricated and one should do: user_url(1, :format => :js). Every route has the :format segment, which rarely gets used (only with non-html formats). Checking for route_segments.size would be useless.

  Finally, I'm not sure if your implementation will work on views and helpers.

  That is a big concern, that I've forgot. I'll look into it.

• 

  José Valim January 10th, 2009 @ 01:51 PM

  My point about "all applications are doing like that" is because your current implementation will break what our application are doing now, it breaks api.

  For example, in the moment I define default_url_options in my controller with :locale, all route methods that I'm passing the locale as parameter will break.

  Oh, and I totally forgot that Rails 2.3 changed to have :format as an optional parameter, great point.

• You flagged this item as spam.
  

  iain January 10th, 2009 @ 04:06 PM

  I adjusted the patch a bit, to work in views too.

  Furthermore, I pass the options passed to user_url to default_url_options, so you can do something with that. The code in the eval gets a bit out of hand. It's now long and big. Blegh.

  Your problem didn't make any sense to me, but when I finally worked out a more sensible example, I finally began to see the problem ;)

  
  map.foo 'my_foo/:foo/:bar', :controller => 'users', :action => 'do_foo'
  
  def default_url_options(options = {})
    { :foo => 'bang' }
  end
  
  foo_url(foo, bar)
  

  This would raise an error, because foo would fill :bar and bar would be too much. But with the :format option in routes, I cannot check for it anymore. It's either working for custom routes, or for resources. If format is such a special case, I could leave it out of the equation:

  
  if (route_segments - [:format]).size > args.size .....
  

  Don't know how you feel about that though.

  I have included a new patch, including the changes I mentioned above.

  • default_url_options-2.patch 21.5 KB
• 

  José Valim January 10th, 2009 @ 05:26 PM

  Great, but why don't you try to use defined?. It would simplify things a little bit:

  
  default_segments = if defined?(default_url_options)
    default_url_options
  elsif defined?(controller.default_url_options)
    controller.default_url_options
  else
    nil
  end
  
  if default_segments.is_a?(Hash)
    route_segments -= default_segments.symbolize_keys.keys
  end
  

  But I'm not sure if it won't break anything. I've already tried to solve this ticket and my first attempt was to make :locale (and only :locale) as optional (you can see the patch in this thread). It was not accepted because it's a very specific approach.

  Then when I attempted to make it "generic", as the one you are doing now, I couldn't get it done quickly, I ran out of time. Our biggest problem is that we are trying to guess which parameters are optional and guessing at this point could be dangerous.

  Maybe the best solution would be to specify (not guess) that a segment is optional by assigning a default value. Rails has such API:

  
    map.connect ':controller/:action/:id', :path_prefix => ':locale', :defaults => { :locale => 'en' }
  

  Then we just think that we have to overwrite :locale using default_url_options and everytihg will be solved. But we face some problems:

  1. The hash defined in default_url_options will NOT overwrite the one specified in :defaults.

  2. The segment specified as default cannot be followed by a dynamic segment. This means that we can't do this:

  
    map.connect ':locale/users/:username', :path_prefix => ':locale', :defaults => { :locale => 'en' }
  

  1. map.resources does not accept :defaults option (as long as I tested it, it was ignored)

  However, I cannot say exactly how many work is needed to solve those problems.

  When Rails :format was made optional, it was just created a new class for it and they didn't had to handle such scenarios.

  That said, maybe you could also create a new class to handle :locale segment. It would probably take some time before it gets into core (if it does) but until there it could be used as plugin. :)

  Sorry for the long reply, I'm just trying to share what I discovered so far about this issue. :)

• You flagged this item as spam.
  

  iain January 12th, 2009 @ 08:42 PM

  Great, but why don't you try to use defined?

  Breaks too much tests... (although I might be able to clean it up a bit)

  Sorry for the long reply

  Fantastic :-)

  I have been mulling it over, and what if the options passed to user_url() are passed to default_url_options as well? This way you can exclude it when needed.

  
  def default_url_options(method, *args)
    options = args.extract_options!
    if method == :user and args.size == 2
      {}
    else
      { :locale => I18n.locale }
    end
  end
  

  This way you can keep your way of doing it, with changing it in one special location, and people start new projects can start it anew, without the check. I can even check if you use this message by checking if you use *args (i.e. a negative arity).

  So, as long as you don't use the splat-operator, no args are removed, if you do, you opt-in for the new way of doing things: every key you specify in default_url_options can only be overridden when it is specified specifically in the user_url method.

  This is an API change though, but a sane one I think.

• You flagged this item as spam.
  

  xdmx February 16th, 2009 @ 02:26 PM

  will this be included in the upcoming rails 2.3 or will we wait until the 3.0 or some 2.3.x?

• You flagged this item as spam.
  

  Brendan Baldwin February 18th, 2009 @ 12:53 AM

  Wonder if maybe we could extend the routes DSL to support exclusion of some route segment params from the list of positional arguments altogether?

• You flagged this item as spam.
  

  Brendan Baldwin February 18th, 2009 @ 01:03 AM

  Could do something like:

  map.with_options :prefix => ':locale',
                   :skip_arguments => [:locale] do |loc|
    loc.resources :things
  end
  
  

  Or force/rework the positional arguments order:

  map.with_options :prefix => ':locale' do |loc|
    loc.resources :things, :param_order => [:id]
  end
  
  

  In both of these cases, we would still use default_url_options to patch in the locale default, but methods like this would still work:

  thing_url(123)
  
  

• You flagged this item as spam.
  

  Kunal Shah June 10th, 2009 @ 04:44 PM

  I'm using default_url_options to persist a query_parameter throughout all actions and views of a particular controller when that query parameter is initially present. Just so you understand my use case if the parameter is present a popup layout gets rendered instead of the full application layout and some other magic occurs.

  I've had no problem using this EXCEPT for the new resource path named route as in:

  map.resources :posts

  new_post_path => does not retain the default url options

  Any updates on this?