WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 5 months ago

#35357 accepted enhancement

Introduce did_filter()

Reported by: mordauk Owned by: chriscct7
Milestone: Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

I think there should a did_filter( $tag ) function that mimics the existing did_action( $tag ) function.

While actions and filters are pretty much the same thing, it turns out that you can't actually use did_action() to detect when a filter has been run.

function did_filter( $tag = '' ) {
    global $wp_filter;

    if ( ! isset( $wp_filter[ $tag ] ) ) {
        return 0;
    }

    return $wp_filter[ $tag ];
}

Attachments (6)

35357.patch (752 bytes) - added by mordauk 4 years ago.
35357.2.patch (6.6 KB) - added by chriscct7 4 years ago.
35357.3.patch (4.9 KB) - added by chriscct7 4 years ago.
Removes accidental indentation fixes on the previous patch
35357.php (1.5 KB) - added by chriscct7 4 years ago.
Plugin that uses did_filter based on patch 3
35357.2.php (1.5 KB) - added by chriscct7 4 years ago.
Version using actions
35357.4.patch (6.4 KB) - added by chriscct7 4 years ago.

Download all attachments as: .zip

Change History (23)

@mordauk
4 years ago

#1 @mordauk
4 years ago

  • Keywords has-patch added

#2 @mordauk
4 years ago

  • Keywords has-patch removed

That patch doesn't work for numerous reasons I've now realized but I still think some form of this should happen.

@chriscct7
4 years ago

#3 @chriscct7
4 years ago

  • Keywords has-patch has-unit-tests added
  • Milestone changed from Awaiting Review to 4.5
  • Version trunk deleted

Implemented a method of counting filters as they run similar to to how actions are counted.

If run after a filter is run, returns the number of functions that ran on the filter. If run in a function attached to a filter, returns the number of functions that have run on the filter before the current function. This works the same ways as did_action does, but for filters.

Attached patch, and passing unit tests which test:

  1. Filter with no functions attached
  2. Filter with 1 function attached
  3. Filter with 2 functions attached

Unit tests are passing. Let's get this in 4.5.

#4 @chriscct7
4 years ago

  • Keywords commit added

@chriscct7
4 years ago

Removes accidental indentation fixes on the previous patch

#5 @chriscct7
4 years ago

  • Owner set to chriscct7
  • Status changed from new to assigned

#6 @chriscct7
4 years ago

  • Status changed from assigned to accepted

@chriscct7
4 years ago

Plugin that uses did_filter based on patch 3

#7 follow-up: @nacin
4 years ago

  • Keywords needs-patch added; has-patch has-unit-tests commit removed

If we are to do this, I wonder if it should just be an alias of did_action() and use $wp_actions. That would mimic current_filter() and add_filter() behavior as well, making it clear they share the same namespace.

I've never seen much utility for this, because filters are often run in different, repeated contexts, rather than a single-time action. But I stand corrected and I'm not against adding it.

This requires unit tests, and preferably ones that would catch that the increment is currently in the wrong spot. If you had 4 callbacks tied to the test_did_filter filter, calling apply_filters( 'test_did_filter' ); var_dump( did_filter( 'test_did_filter' ); would return 4, not 1.

If run after a filter is run, returns the number of functions that ran on the filter. If run in a function attached to a filter, returns the number of functions that have run on the filter before the current function. This works the same ways as did_action does, but for filters.

That is not how did_action() works. :-)

#8 @mordauk
4 years ago

The instance that made me open this was when I wanted to detect if a particular filter had run and then prevent it from ever running again. Definitely a rare scenario but it was one that made sense in my particular case.

#9 in reply to: ↑ 7 @chriscct7
4 years ago

A use case of the proposed function is to allow a function attached to a filter to detect that another function has run and shortcircuit. For example, if you had a filter that could be called multiple times to avoid a loop.

Replying to nacin:

If we are to do this, I wonder if it should just be an alias of did_action() and use $wp_actions. That would mimic current_filter() and add_filter() behavior as well, making it clear they share the same namespace.

I've never seen much utility for this, because filters are often run in different, repeated contexts, rather than a single-time action. But I stand corrected and I'm not against adding it.

This requires unit tests, and preferably ones that would catch that the increment is currently in the wrong spot. If you had 4 callbacks tied to the test_did_filter filter, calling apply_filters( 'test_did_filter' ); var_dump( did_filter( 'test_did_filter' ); would return 4, not 1.

If run after a filter is run, returns the number of functions that ran on the filter. If run in a function attached to a filter, returns the number of functions that have run on the filter before the current function. This works the same ways as did_action does, but for filters.

That is not how did_action() works. :-)

If I adjust the PHP file I uploaded above to use actions instead of filters, that's how it seems to work. Attaching a copy.

@chriscct7
4 years ago

Version using actions

#10 follow-up: @chriscct7
4 years ago

I stand corrected. It does indeed return 1. Therefore there is a difference, however I don't see a use case where the way did_action which counts per do_action call as opposed to the number of functions that run on it (like my above patch does) makes sense for filters. I actually think the patch method of counting is more expected for filters because of exactly this:

I've never seen much utility for this, because filters are often run in different, repeated contexts, rather than a single-time action.

Last edited 4 years ago by chriscct7 (previous) (diff)

#11 @chriscct7
4 years ago

With filters I think its more interesting to count the number of functions that have run on it, rather than the number of times a filter has been called, and that's what the patch was designed to solve (thus why the unit tests passed). Despite apparently being misconcieved on what did_action does, the application of 4 functions to a single filter should return 4 based on what I've written if did_filter is run after the filter, and 0 , 1, 2, and 3 respectively if run inside each of the functions.

Perhaps a different function name for it, and then something like https://core.trac.wordpress.org/attachment/ticket/35357/35357.patch for the did_filter

#12 in reply to: ↑ 10 @dd32
4 years ago

Replying to chriscct7:

I stand corrected. It does indeed return 1. Therefore there is a difference, however I don't see a use case where the way did_action which counts per do_action call as opposed to the number of functions that run on it (like my above patch does) makes sense for filters. I actually think the patch method of counting is more expected for filters because of exactly this:

I've never seen much utility for this, because filters are often run in different, repeated contexts, rather than a single-time action.

Just to throw another opinion at it

  • I don't see the point of counting the number of functions that've run.
  • I don't think "detecting another function has already run on this filter" is a valid use-case that should be supported. If a filter *shouldn't* run, it should either be remove_action()'d or not hooked in the first place if another filter is already hooked in.
  • As filters are more likely to be called multiple times per page load it feels more sane that it'd return the number of times that filter has run on the current load, "the_title has run 15 times" is a lot more useful than "the_title has had 150 functions run on it".

#13 @mordauk
4 years ago

I needed to detect if a filter had been applied because the callback function tied to the filter had the possibility of triggering an infinite loop since it resulting in the class that ran the filter getting instantiated again.

#14 @dd32
4 years ago

I needed to detect if a filter had been applied because the callback function tied to the filter had the possibility of triggering an infinite loop since it resulting in the class that ran the filter getting instantiated again.

In that case, I think something like doing_filter( 'the_filter_name' ) is a better option

@chriscct7
4 years ago

#15 @chriscct7
4 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

In 35357.4.patch:

  • Adjusted did_filter to work similar to did_action to count the number of times a filter is called by having it wrap did_action and adjusting the filter run functions to count the times a filter is called the same way they do in the action call functions
  • Adjusted did_filter unit tests to test the number of times a filter that is called 1 and 2 times and one that is never called
  • Introduced filter_functions_run to count the number of functions run on a filter
  • Adjusted filter_functions_run unit tests to do what patch 3's unit tests did.
Last edited 4 years ago by chriscct7 (previous) (diff)

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 years ago

#17 @jorbin
4 years ago

  • Milestone changed from 4.5 to Future Release

7 weeks since the last activity, so this is being punted.

Note: See TracTickets for help on using tickets.