Fixes skipping object callback filters #8687

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

benmcredmond commented Jan 2, 2013

This allows you to skip callbacks that are defined by objects, e.g. for ActionController:

skip_after_filter MySpecialFilter

Previously this didn't work due to a bug in how Rails compared callbacks in Callback#matches?. When a callback is compiled, if it's an object filter (i.e. not a method, proc, etc.), Callback now defines a method on @klass that is derived from the class name rather than @callback_id. So, when skip_callback tries to find the appropriate callback to remove, Callback can regenerate the method name for the filter object and return the correct value for Callback#matches?.

@benmcredmond benmcredmond Fixes skipping object callback filters
This allows you to skip callbacks that are defined by objects, e.g. for
`ActionController`:

    skip_after_filter MySpecialFilter

Previously this didn't work due to a bug in how Rails compared callbacks
in `Callback#matches?`. When a callback is compiled, if it's an object
filter (i.e. not a method, proc, etc.), `Callback` now defines a method on
`@klass` that is derived from the class name rather than `@callback_id`.
So, when `skip_callback` tries to find the appropriate callback to
remove, `Callback` can regenerate the method name for the filter
object and return the correct value for `Callback#matches?`.
16c5c55

benmcredmond referenced this pull request in intercom/intercom-rails Jan 2, 2013

Closed

Can't skip auto include filter #20

Please send your pull request against master, if it gets merged it might be backported if necessary. We don't usually merge things in 3-2-stable first. Thanks!

Contributor

benmcredmond commented Jan 7, 2013

Right, will do. Cheers.

On Thu, Jan 3, 2013 at 10:55 AM, Carlos Antonio da Silva <
notifications@github.com> wrote:

Please send your pull request against master, if it gets merged it might
be backported if necessary. We don't usually merge things in 3-2-stable
first. Thanks!


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/8687#issuecomment-11855804.

Does this pull request need to remain open? (If it was submitted against master, I couldn't find it.)

Contributor

benmcredmond commented Feb 8, 2013

nope, been meaning to make against master haven't got to it yet. Will close.

On Fri, Feb 8, 2013 at 10:49 AM, Wally Altman notifications@github.comwrote:

Does this pull request need to remain open? (If it was submitted against
master, I couldn't find it.)


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/8687#issuecomment-13305060..

Contributor

stouset commented Apr 1, 2013

@benofsky Still planning to close/rebase? :)

Contributor

benmcredmond commented Apr 1, 2013

sorry, yes, will do today.

On Mon, Apr 1, 2013 at 12:00 PM, Stephen Touset notifications@github.comwrote:

@benofsky https://github.com/benofsky Still planning to close/rebase? :)


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/8687#issuecomment-15730728
.

Contributor

benmcredmond commented Apr 1, 2013

re-opened against master #10034

Contributor

benmcredmond commented Apr 3, 2013

I did this yesterday: #10034

On Mon, Apr 1, 2013 at 12:05 PM, Ben McRedmond ben@benmcredmond.com wrote:

sorry, yes, will do today.

On Mon, Apr 1, 2013 at 12:00 PM, Stephen Touset notifications@github.comwrote:

@benofsky https://github.com/benofsky Still planning to close/rebase?
:)


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/8687#issuecomment-15730728
.

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