Support unicode character route directly in config/routes.rb. Fix #3470. #6646

Merged
merged 1 commit into from Jun 15, 2012

Conversation

Projects
None yet
5 participants
Contributor

kennyj commented Jun 6, 2012

Support unicode character route in config/routes.rb, e.g.:

get 'こんにちは', :controller => 'home', :action => 'index'

In current Rails, we had to draw encoded route, e.g.:

get Rack::Utils.escape('こんにちは'), :controller => 'home', :action => 'index'

Please see also #3470

Member

steveklabnik commented Jun 6, 2012

Mentioning #3470 so GitHub links them.

Contributor

kennyj commented Jun 6, 2012

@steveklabnik
thanks !
I should use #xxxx instead of url.

Member

steveklabnik commented Jun 6, 2012

Yep! You can also link to another repo's issue by user/repo#number

Member

drogus commented Jun 8, 2012

@kennyj this breaks backwards compatibility because:

#encoding: utf-8
require 'rack/utils'
p Rack::Utils.escape('こんにちは')
#=> "%E3%81%93%E3%82%93%E3%81%AB%E3%81%A1%E3%81%AF"
p Rack::Utils.escape(Rack::Utils.escape('こんにちは')) 
#=> "%25E3%2581%2593%25E3%2582%2593%25E3%2581%25AB%25E3%2581%25A1%25E3%2581%25AF"

If someone escapes paths already, their rotes will break. I'm not saying that this is the thing that prevents us from merging it, just pointing out the problem. Maybe an addition to upgrading guide will be sufficient.

Contributor

kennyj commented Jun 10, 2012

@drogus I knew it, and I think my PR is proposal one.

I'm going to add an entry on https://github.com/rails/rails/blob/master/guides/source/upgrading_ruby_on_rails.textile.

Contributor

kennyj commented Jun 10, 2012

done !

Since my English is poor, If my entry is not good English, please tell me ;0

kuraga commented Jun 10, 2012

@kennyj Thank you! Should we write something for API-docs? My English is too bad :)

Contributor

kennyj commented Jun 10, 2012

Should we add an entry on http://guides.rubyonrails.org/routing.html ?
Are there any other ?

Contributor

kennyj commented Jun 12, 2012

@drogus @kuraga I added some docs.

actionpack/CHANGELOG.md
@@ -1,5 +1,15 @@
## Rails 4.0.0 (unreleased) ##
+* Support unicode character route in config/routes.rb, e.g.:
@carlosantoniodasilva

carlosantoniodasilva Jun 13, 2012

Owner

I don't think it's necessary to talk about "config/routes.rb".. Only routes should suffice. How about showing the old way before?

Support unicode characters in routes, so instead of manually escaping:

    get Rack::Utils.escape('こんにちは') => 'home#index'

It's possible to write the unicode route and Rails will automatically escape it:

    get 'こんにちは' => 'home#index'
@kuraga

kuraga Jun 14, 2012

Yes, you're right.

@kuraga

kuraga Jun 14, 2012

What do you think about an idea to highlight that the argument will be automatically escaped (see #6646 (comment); how to insert comment's link?) This changelog means that I may don't escape unicode characters. But I must don't do it...

@carlosantoniodasilva

carlosantoniodasilva Jun 14, 2012

Owner

Yeah sounds good. Can remove the It's possible to write the unicode route to something like Just write the unicode route, or similar :).

@kuraga

kuraga Jun 14, 2012

Support unicode characters in routes. Route will be automatically escaped, so instead of manually escaping:

    get Rack::Utils.escape('こんにちは') => 'home#index'

You just have to write the unicode route:

    get 'こんにちは' => 'home#index'
@kennyj

kennyj Jun 14, 2012

Contributor

@kuraga @carlosantoniodasilva thanks !
I'm updating CHANGELOG.

p.s. こんにちは means hello in Japanese. It's safety word :D

@carlosantoniodasilva

carlosantoniodasilva Jun 15, 2012

Owner

Nice, I feel like I have to learn Japanese :)

I think we're fine to have a note on the upgrading guides and the changelog, showing people they don't need to escape routes anymore manually. It's a quick change to do, in only one file, in case they are.

kuraga commented Jun 14, 2012

Thank you!!! I'm very glad to take part of creating first Rails' change of my life!!! I'm hope for accepting this PR. And thank you about translation of 'こんにちは' :)

Member

drogus commented Jun 14, 2012

@kennyj sorry for delay, I looked on guides entry that you added and it looks good

Member

drogus commented Jun 14, 2012

@kennyj or, actually, one really small thing, I would change "Rails 4.0 also changed how draws unicode character routes." to "Rails 4.0 also changed the way unicode character routes are drawn."

actionpack/CHANGELOG.md
@@ -1,5 +1,15 @@
## Rails 4.0.0 (unreleased) ##
+* Support unicode characters in routes. Route will be automatically escaped, so instead of manually escaping:
+
+ get Rack::Utils.escape('こんにちは') => 'home#index'
@kuraga

kuraga Jun 14, 2012

Sorry but one more thing.
I think line of code should be highlighted by spaces by left?

actionpack/CHANGELOG.md
+
+ You just have to write the unicode route:
+
+ get 'こんにちは' => 'home#index'
@kuraga

kuraga Jun 14, 2012

...And here...

Contributor

kennyj commented Jun 15, 2012

@drogus @kurage Thank you. I'm updating it ... done !

guides/source/upgrading_ruby_on_rails.textile
@@ -50,6 +50,8 @@ h4(#action_pack4_0). Action Pack
Rails 4.0 changed how <tt>assert_generates</tt>, <tt>assert_recognizes</tt>, and <tt>assert_routing</tt> work. Now all these assertions raise <tt>Assertion</tt> instead of <tt>ActionController::RoutingError</tt>.
+Rails 4.0 also changed the way unicode character routes are drawn. Now you can draw unicode character routes directly. If you already draw these, you must change these, e.g. <tt>get Rack::Utils.escape('こんにちは'), :controller => 'welcome', :action => 'index'</tt> to <tt>get 'こんにちは', :controller => 'welcome', :action => 'index'</tt>.
@carlosantoniodasilva

carlosantoniodasilva Jun 15, 2012

Owner

I think we can just change this slightly, how about:

If you already draw these, you must change these, => If you already draw such routes, you must change them,

@kennyj

kennyj Jun 15, 2012

Contributor

@carlosantoniodasilva @kuraga thanks ! I agree with you, and I updated it !

carlosantoniodasilva added a commit that referenced this pull request Jun 15, 2012

Merge pull request #6646 from kennyj/fix_3470
Support unicode character route directly in config/routes.rb. Fix #3470.

@carlosantoniodasilva carlosantoniodasilva merged commit 9abe098 into rails:master Jun 15, 2012

@kennyj merged, thanks!

kuraga commented Jun 15, 2012

Great! Thanks!!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment