Make WordPress Core

Opened 18 years ago

Closed 17 years ago

#5232 closed enhancement (fixed)

Get the current action and current filter name

Reported by: tellyworth's profile tellyworth Owned by:
Milestone: 2.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

Currently there's no easy way for a filter to know the name of the current filter. Likewise for actions. That's not a problem if you're only filtering on a single tag, but it could be useful to know if you're applying a function to multiple actions/filters, or using the 'all' filter.

The enclosed patch sets a global $wp_current_action or $wp_current_filter with the tag name of the current action/filter. Using a global seems a little rough, but it could be improved if the concept is sound.

Attachments (3)

wp-current-filter-r6720.patch (1.5 KB) - added by tellyworth 18 years ago.
wp-current-filter-revised-r6278.patch (1.8 KB) - added by tellyworth 18 years ago.
wp-current-filter-revised-r6317.patch (2.3 KB) - added by tellyworth 17 years ago.

Download all attachments as: .zip

Change History (17)

#1 @darkdragon
18 years ago

Why would you need to know this?

#2 @tellyworth
18 years ago

For debugging, logging, profiling, or for one one filter or action function is attached to multiple filters, as the description says.

#3 @foolswisdom
18 years ago

  • Milestone changed from 2.5 to 2.4

#4 @westi
18 years ago

-1 to adding another global.

What real-world problem does this global pollution solve?

#5 @santosj
18 years ago

I'm planning on classes which might allow for this.

#6 @mdawaffe
18 years ago

I recently wrote a plugin that filtered both the_content and comment_text. The filtration was dependent on what other filters were attached to those hooks.

if ( kses is attached to $this_hook ) {
  do this;
} else {
  do something else;
}

Since there's no way to know what $this_hook is in the above code, I was forced to write two functions: one with $this_hook hardcoded as the_content, and one doing the same with comment_text.

Knowing what the current filter was would have made the code more efficient.

As tellyworth also mentions, this knowledge can also be useful in debugging and unit testing.

I'm also not a big fan of the global idea, but I like the feature. We could use a static variable in a function that both sets and gets:

// Sets if passed one parameter
// Gets if passed no parameters
function wp_current_filter() {
  static $current = false;

  if ( func_num_args() )
    $current = func_get_arg(0);

  return $current;
}

function apply_filters( $tag, $string ) {
  ...
  // Set current filter
  wp_current_filter( $tag );
  ...
}

function my_filter( $string ) {
  // Get current filter
  $current_filter = wp_current_filter();
  ...
  return $string;
}

#7 @tellyworth
18 years ago

Another approach that I tried but rejected for simplicity's sake was to use an array and pop/push, so it's possible to see the stack of actions when they are nested. That could be combined with mdawaffe's static approach.

#8 @darkdragon
18 years ago

The static approach would have to work like the global one where after the action is take, it sets the current action to the old one. The reason for this is for hooks that call other hooks. You would lose the previous information.

So +1 for mdawaffe's solution, but only if it is combined with tellyworth's.

#9 @tellyworth
18 years ago

I wrote a few draft patches for this yesterday, and after reviewing those I came to two conclusions:

  1. The filter/action code is time critical. Anything that adds an extra function call, object dereference or similar can have a significant impact on performance.
  1. Properly encapsulating the current_filter data requires a singleton object; a function with a static variable is messy and involves several branches and function calls (which affect performance). Short of refactoring the filter/action API, I can't see a good way to overcome that.

wp-current-filter-revised-r6278.patch is a compromise. It uses only one global, and provides a current_filter() API function for fetching the filter or action name. Only 2 lines of code are needed in each of the time critical functions, with no branching or user function calls.

#10 @darkdragon
18 years ago

Agreed, I had to overcome a similar limitation while developing a patch. One of the problems perhaps is that every millisecond adds up when you need to have calls totaling over 3000 times. I'm unsure what the total is WordPress, but in which case the upper limit before things start running really slow (1-3 seconds) is around 4000 to 6000 times.

The problem is that in most cases when you do Object Oriented code, you'll want to do the time consuming tests once and before the hook is added. That way, you can just focus on calling what you need without doing the checks in the call method.

I've done some prototyping with this (all complete crap). I was thinking about it. That the code would be a really good candidate for a PHP extension. The code is simple enough, that it wouldn't take more than a few days to write, test, and deploy. The only problem is that you'll need something that everyone would have and not many would go through the effort to install a PECL extension.

I'm not so much concern with the objects as I am with having to support PHP4 with such an object. I'll be unwilling to write OO code that has to be compatible with PHP4.

The patch is beautiful by the way. I really do think it should be accepted.

#11 @ryan
17 years ago

Needs update now that #5231 went in.

#12 @tellyworth
17 years ago

the revised-r6317 patch is a quick update to work with #5231.

It still uses a single $wp_current_filter global to store the current filter or action name. Perhaps it's worth improving it to store the filter and action separately.

#13 @santosj
17 years ago

Given the execution of PHP and the pushing and popping, I don't think it is worth it. You could put some metadata in along with the tag to say whether it is an action or a filter. However, creating a separate global doesn't seem logical.

#14 @ryan
17 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [6318]) current_fiter() from tellyworth. fixes #5232

Note: See TracTickets for help on using tickets.