Rails Ruby on Rails → 3.0.6

Comments and changes to this ticket

• 

  Prem Sichanugrist (sikachu) March 22nd, 2011 @ 03:36 PM

  This is a patch for master.

  I'm still working on the patch for 3-0-stable, as my patch is currently rely on these two commits: https://github.com/rails/rails/compare/53794cf...acd4bfb. I think those commits should be pull to 3-0-stable as well.

  Would you guys give me any though on that? If you think that commit should be left there, I'll then cook a separate patch for 3-0-stable.

  • hide_optional_format-master.patch 2 KB
• 

  Repository March 22nd, 2011 @ 04:20 PM

  • State changed from “verified” to “resolved”

  (from [2ddfdba9a0dab7d8499c3ad0d13583bddbac4f69]) Do not show optional (.:format) block for wildcard route [#6605 state:resolved]

  This will make the output of rake routes to be correctly match to the behavior of the application, as the regular expression used to match the path is greedy and won't capture the format part by default
  https://github.com/rails/rails/commit/2ddfdba9a0dab7d8499c3ad0d1358...

• 

  Andrew White March 22nd, 2011 @ 04:33 PM

  • State changed from “resolved” to “verified”

  A couple of things to note - firstly the glob param may have other elements before it and secondly you may have multiple glob params in a path. You'd be better off checking that the last segment in a path is a glob parameter, e.g:

    path.match(/\*[^\/]+$/)
  

  I think the safe option for stable is to not add the format for glob paths, but I think we should fix master to add a non-greedy regexp if the path ends with a glob parameter.

• 

  Repository March 22nd, 2011 @ 05:11 PM

  • State changed from “verified” to “resolved”

  (from [2ef6270f8fbbefba8d4f10504497e198d8e7deea]) Merge branch 'master' into fuuu

  • master: Do not show optional (.:format) block for wildcard route [#6605 state:resolved] pushing id insertion and prefetch primary keys down to Relation#insert use prepared statements to fetch the last insert id escaping binary data encoding when inserting to sqlite3. Thanks Naruse! [#6559 state:resolved] schemas set by set_table_name are respected by the mysql adapter. [#5322 state:resolved] Reapply extensions when using except and only SJIS is an alias to Windows-31J in ruby trunk. Use SHIFT_JIS for this test Improved resolver docs a bit [action_view] docs for FileSystemResolver [action_view] added custom patterns to template resolver https://github.com/rails/rails/commit/2ef6270f8fbbefba8d4f10504497e...
• 

  Prem Sichanugrist (sikachu) March 23rd, 2011 @ 01:13 AM

  • State changed from “resolved” to “verified”

  @Andrew White: but then the RegExp actually has to be fixed in Rack::Mount, right?

• 

  Andrew White March 23rd, 2011 @ 10:33 AM

  There's a simple fix required in Rack::Mount - it ignores custom conditions on glob params. Once that is fixed we can use the regexp above to detect whether the last segment is a glob param and then add a non-greedy regexp condition for that param if one isn't specified. That way all of the responds_to magic should work.

• 

  Prem Sichanugrist (sikachu) March 23rd, 2011 @ 11:04 AM

  If it's a simple fix, then I'm totally agree with you. Are you going to fix that in Rack::Mount for me, or do you want me to send pull request in?

  Anyhow, here's what I'm going to do:

  1. Backport the patch to 3-0-stable so people would notice it as a bugfix. With appropriate CHANGELOG in it.
     
  2. Waiting for the release of new version of Rack::Mount
     
  3. Update the patch on master to not filter out the (.:format) part, and add the CHANGELOG to say that the bug on not detecting format route is fixed on Rack::Mount.

  Do you think that looks ok?

• 

  Prem Sichanugrist (sikachu) March 23rd, 2011 @ 11:30 AM

  By the way, I'm backing of on pulling those two Aaron's patch back, as I don't think user would expect that on a stable branch. I don't think I want to break someone else's application

  (I'm also adding Aaron to the ticket now so he know what's my plan of world domination ..)

• 

  Andrew White March 23rd, 2011 @ 12:12 PM

  Okay, I've sent a pull request to Josh: https://github.com/josh/rack-mount/pull/15

  Since the :format parameter is Rails-specific we need to specify the non-greedy regexp in the route we pass to Rack::Mount. We need to bump the version of rack-mount to the new release on master and then add a CHANGELOG entry to say that glob parameters at the end of a route are non-greedy by default and if you want the extension included in the parameter then pass :format => false in the route options hash.

  For stable I'd not change anything and if someone updates to the newer version of rack-mount they can add the non-greedy regexp and it will work as expected.

• 

  Repository March 29th, 2011 @ 10:14 AM

  • State changed from “verified” to “resolved”

  (from [51551a0a5bd6f7e4116bc3759a4d7c15634643dc]) Update the wildcard route to be non-greedy by default, therefore be able to match the (.:format) segment [#6605 state:resolved]

  After some discussion with Andrew White, it seems like this is a better approach for handling a wildcard route. However, user can still bring back the old behavior by supplying :format => false to the route.

  Signed-off-by: Andrew White andyw@pixeltrix.co.uk
  https://github.com/rails/rails/commit/51551a0a5bd6f7e4116bc3759a4d7...