Bunch of patches

rubyonrails-core
1

Hey guys, I’ve pushed a bunch of patches and tickets today:

https://rails.lighthouseapp.com/projects/8994/tickets/5969-bump-i18n-and-make-translationhelper-use-new-rescue_format-option#ticket-5969-8

https://github.com/svenfuchs/rails/commit/a0f059618cb6eb46dfb97ef49004da776bf00d4c

With I18n 0.5.0 we’ll ship some improvements to the exception api. This patch makes the translation view helper leverage those improvements (by not catching exceptions itself any more).

https://rails.lighthouseapp.com/projects/8994/tickets/6013-move-19-string-interpolation-syntax-backport-from-i18n-to-activesupport

https://github.com/svenfuchs/rails/commit/85a05662fe7e0b06e9c7cd93cae9c3721058d879

We’ve shipped a backport of 1.9 string interpolation syntax for quite some versions in I18n. Now we’re going to not use String#% anymore but implement our own helper method for it. Meanwhile Rails (and probably others) has started relying on this behavior and it seems appropriate to move this to ActiveSupport (should probably have been done right from the beginning).

https://rails.lighthouseapp.com/projects/8994/tickets/6027-define-respond_to-on-actiondispatchintegrationrunner

https://github.com/svenfuchs/rails/commit/64438c2987b6ead8ec28f5962d7d43fa7f01bc3c

This is just a small bug. ActionDispatch::Integration::Runner implements method_missing but misses respond_to?. This can make tests fail where application code relies on respond_to?

https://rails.lighthouseapp.com/projects/8994/tickets/2986-polymorphic_url-should-handle-sti-better#ticket-2986-13

https://github.com/svenfuchs/rails/commit/2d298f2a140cdc8388bf09ce517dd73b698c7a4e

Ticket was opened by Luke Melia in August. I’ve now extracted an implementation from adva-cms2 that solves the problem and also seems to speed up polymorphic_url due to caching. Caching still needs to be tweaked though.

https://rails.lighthouseapp.com/projects/8994/tickets/1975-allow-to-register-javascriptstylesheet_expansions-to-existing-symbols#ticket-1975-15

https://github.com/svenfuchs/rails/commit/b712476ac3f20d1e976ea431ccf44827c419a281

A ticket from the stone age when engines still were a rather obscure thing and people failed to see a usecase. We clearly have a need for this and thus I’m trying to bump this once again. The patch doesn’t break bc.

My last tickets were lying around for quite a while so I’m trying to raise some attention over here.

2

Thanks Sven! I'll look through these today.

3

Awesome :slight_smile:

I think I've added a note in the ticket about which branch I've patched against. I'm usually not quite sure about this. Lemme know if something needs to be rebased or something. (I'm hanging out on irc, too.)

4

Hey guys, I've pushed a bunch of patches and tickets today:

#5969 Bump I18n and make TranslationHelper use new :rescue_format option - Ruby on Rails - rails Make TranslationHelper#translate use the :rescue_format option in I18… · svenfuchs/rails@a0f0596 · GitHub With I18n 0.5.0 we'll ship some improvements to the exception api. This patch makes the translation view helper leverage those improvements (by not catching exceptions itself any more).

Can you rebase this one from master? It didn't apply cleanly for me.

#6013 Move 1.9 string interpolation syntax backport from I18n to ActiveSupport - Ruby on Rails - rails Move 1.9 string interpolation syntax backport from I18n to ActiveSupport · svenfuchs/rails@85a0566 · GitHub We've shipped a backport of 1.9 string interpolation syntax for quite some versions in I18n. Now we're going to not use String#% anymore but implement our own helper method for it. Meanwhile Rails (and probably others) has started relying on this behavior and it seems appropriate to move this to ActiveSupport (should probably have been done right from the beginning).

I'm still working on this. I'd like to get this patch in the backports gem. I'm not sure which is more appropriate, AS, or backports:

  GitHub - marcandre/backports: The latest features of Ruby backported to older versions.

#6027 Define respond_to? on ActionDispatch::Integration::Runner - Ruby on Rails - rails add respond_to? to ActionDispatch::Integration::Runner · svenfuchs/rails@64438c2 · GitHub This is just a small bug. ActionDispatch::Integration::Runner implements method_missing but misses respond_to?. This can make tests fail where application code relies on respond_to?

This is applied.

#2986 polymorphic_url should handle STI better - Ruby on Rails - rails Make polymorphic_url walk up the inheritance chain for given records … · svenfuchs/rails@2d298f2 · GitHub Ticket was opened by Luke Melia in August. I've now extracted an implementation from adva-cms2 that solves the problem and also seems to speed up polymorphic_url due to caching. Caching still needs to be tweaked though.

#1975 Allow to register javascript/stylesheet_expansions to existing symbols - Ruby on Rails - rails Allow registering javascript/stylesheet_expansions to existing symbols · svenfuchs/rails@b712476 · GitHub A ticket from the stone age when engines still were a rather obscure thing and people failed to see a usecase. We clearly have a need for this and thus I'm trying to bump this once again. The patch doesn't break bc.

I'm still working on these two. :slight_smile:

5

#5969 Bump I18n and make TranslationHelper use new :rescue_format option - Ruby on Rails - rails Make TranslationHelper#translate use the :rescue_format option in I18… · svenfuchs/rails@a0f0596 · GitHub With I18n 0.5.0 we'll ship some improvements to the exception api. This patch makes the translation view helper leverage those improvements (by not catching exceptions itself any more).

Can you rebase this one from master? It didn't apply cleanly for me.

Sure. Will do that tomorrow.

#6013 Move 1.9 string interpolation syntax backport from I18n to ActiveSupport - Ruby on Rails - rails Move 1.9 string interpolation syntax backport from I18n to ActiveSupport · svenfuchs/rails@85a0566 · GitHub We've shipped a backport of 1.9 string interpolation syntax for quite some versions in I18n. Now we're going to not use String#% anymore but implement our own helper method for it. Meanwhile Rails (and probably others) has started relying on this behavior and it seems appropriate to move this to ActiveSupport (should probably have been done right from the beginning).

I'm still working on this. I'd like to get this patch in the backports gem. I'm not sure which is more appropriate, AS, or backports:

GitHub - marcandre/backports: The latest features of Ruby backported to older versions.

Oh, I didn't even know about that. Is this required by Rails though? At least ActionPack relies on that String#% patch (routes behavior that uses the %{foo} syntax, iirc) but doesn't seem to require the Backports gem? In that case I'd guess it would need to go into ActiveSupport.

#6027 Define respond_to? on ActionDispatch::Integration::Runner - Ruby on Rails - rails add respond_to? to ActionDispatch::Integration::Runner · svenfuchs/rails@64438c2 · GitHub This is just a small bug. ActionDispatch::Integration::Runner implements method_missing but misses respond_to?. This can make tests fail where application code relies on respond_to?

This is applied.

Thank you :slight_smile:

#2986 polymorphic_url should handle STI better - Ruby on Rails - rails Make polymorphic_url walk up the inheritance chain for given records … · svenfuchs/rails@2d298f2 · GitHub Ticket was opened by Luke Melia in August. I've now extracted an implementation from adva-cms2 that solves the problem and also seems to speed up polymorphic_url due to caching. Caching still needs to be tweaked though.

#1975 Allow to register javascript/stylesheet_expansions to existing symbols - Ruby on Rails - rails Allow registering javascript/stylesheet_expansions to existing symbols · svenfuchs/rails@b712476 · GitHub A ticket from the stone age when engines still were a rather obscure thing and people failed to see a usecase. We clearly have a need for this and thus I'm trying to bump this once again. The patch doesn't break bc.

I'm still working on these two. :slight_smile:

Anything I can help with here?

6

We've used the backports gem as inspiration or source for some backports in AS, but it is not a Rails dependency.

7

I think if AP was relying on behavior supplied by i18n, we should continue to use i18n for that support. I mean, we should switch it to use the new method in i18n.

I'm only worried about end users that inadvertently used this backport.

8

Hey guys, I've pushed a bunch of patches and tickets today:

#5969 Bump I18n and make TranslationHelper use new :rescue_format option - Ruby on Rails - rails Make TranslationHelper#translate use the :rescue_format option in I18… · svenfuchs/rails@a0f0596 · GitHub With I18n 0.5.0 we'll ship some improvements to the exception api. This patch makes the translation view helper leverage those improvements (by not catching exceptions itself any more).

#6013 Move 1.9 string interpolation syntax backport from I18n to ActiveSupport - Ruby on Rails - rails Move 1.9 string interpolation syntax backport from I18n to ActiveSupport · svenfuchs/rails@85a0566 · GitHub We've shipped a backport of 1.9 string interpolation syntax for quite some versions in I18n. Now we're going to not use String#% anymore but implement our own helper method for it. Meanwhile Rails (and probably others) has started relying on this behavior and it seems appropriate to move this to ActiveSupport (should probably have been done right from the beginning).

#6027 Define respond_to? on ActionDispatch::Integration::Runner - Ruby on Rails - rails add respond_to? to ActionDispatch::Integration::Runner · svenfuchs/rails@64438c2 · GitHub This is just a small bug. ActionDispatch::Integration::Runner implements method_missing but misses respond_to?. This can make tests fail where application code relies on respond_to?

#2986 polymorphic_url should handle STI better - Ruby on Rails - rails Make polymorphic_url walk up the inheritance chain for given records … · svenfuchs/rails@2d298f2 · GitHub Ticket was opened by Luke Melia in August. I've now extracted an implementation from adva-cms2 that solves the problem and also seems to speed up polymorphic_url due to caching. Caching still needs to be tweaked though.

This looks good. Can you backport the patch to master too? Then I'll apply and push both. Thanks!

#1975 Allow to register javascript/stylesheet_expansions to existing symbols - Ruby on Rails - rails Allow registering javascript/stylesheet_expansions to existing symbols · svenfuchs/rails@b712476 · GitHub A ticket from the stone age when engines still were a rather obscure thing and people failed to see a usecase. We clearly have a need for this and thus I'm trying to bump this once again. The patch doesn't break bc.

Can you backport this patch to master?

9

I’ve rebased all remaining patches to master.

I think these are fine:

Allow registering javascript/stylesheet_expansions to existing symbols

https://github.com/svenfuchs/rails/tree/master-asset_expansions_registration

https://github.com/svenfuchs/rails/commit/514f8f46b2a5facc2fd9aaa31a393c114fbbbe75

Make TranslationHelper#translate use the :rescue_format option in I18n 0.5.0

https://github.com/svenfuchs/rails/tree/master-i18n-0.5.0-translation_helper

https://github.com/svenfuchs/rails/commit/613f0a7ebf4ee93571aef95233ee71a3887ad3d8

Move 1.9 string interpolation syntax backport from I18n to ActiveSupport

https://github.com/svenfuchs/rails/tree/master-backport-1_9-string-interpolation-syntax

https://github.com/svenfuchs/rails/commit/0b4a6d6af31ab165115545ed894a15c0b702c38f

This one still needs some thoughts though:

Make polymorphic_url walk up the inheritance chain for given records

https://github.com/svenfuchs/rails/tree/master-polymorphic_url-sti-fallbacks

https://github.com/svenfuchs/rails/commit/56b0b5a3e1e31bb18ab005ff7f055c4b15a7bc43

See:

https://github.com/svenfuchs/rails/commit/56b0b5a3e1e31bb18ab005ff7f055c4b15a7bc43#L0R135 and

https://github.com/svenfuchs/rails/commit/56b0b5a3e1e31bb18ab005ff7f055c4b15a7bc43#L2R42

As you see I’ve just commented out these two linked lines. It seems this was introduced in https://github.com/rails/rails/commit/613cbe1f0075eed7ebf1ac521e99f652f6891330 I don’t quite get that change, shouldn’t self always respond to the generated named_route helper here? And if it doesn’t, doesn’t that indicate some flaw in a different layer?

Maybe that’s a discussion we should take off-list. I’ll contact Piotr about this :slight_smile: