Add filter to wp_parse_args()

[Viper007Bond]

This will allow for some l33t hackery (basically being able to modify a wide variety of functions).

Think we could squeeze it into 2.9 since it's just a filter?

[Viper007Bond]

Example of the awesomeness of this filter:

I want to control the max_depth value of wp_list_comments(), so here's a function that'll do it:

add_filter( 'wp_parse_args', 'viper_thread_comments_depth', 10, 3 );

function viper_thread_comments_depth( $args, $passed, $defaults ) {
	$wp_list_comments_defaults = array(
		'walker' => null, 'max_depth' => '', 'style' => 'ul', 'callback' => null, 'end-callback' => null, 'type' => 'all',
		'page' => '', 'per_page' => '', 'avatar_size' => 32, 'reverse_top_level' => null, 'reverse_children' => ''
	);

	if ( $wp_list_comments_defaults === $defaults )
		$args['max_depth'] = 1;

	return $args;
}

Obviously that's not 100% reliable for all functions (due to defaults collisions), but there's a ton of cases where it is reliable. :)

[Viper007Bond]

As for performance, I observed only about a 5 millisecond difference (for about 80 calls to the filter), but that was well within the margin of error. In short, it doesn't seem to make any noticeable difference.

[Viper007Bond]

DD32 suggested I stop being lazy and instead just pass the context to wp_list_args() and then call a wp_list_args_CONTEXT filter.

$args = wp_parse_args( $args, $defaults , __FUNCTION__ );

return ( !empty($context) ) ? apply_filters( 'wp_parse_args_' . strtolower($context), $r, $args, $defaults ) : $r;

Works a lot better and is easier for plugin developers to use.

Assuming there's interest in this and it's green lit, I'll start regex'ing WordPress files to add in the __FUNCTION__ call or maybe some weird hybrid.

[Denis-de-Bernardy]

If we pass FUNCTION and wp_parse_args gets used in a plugin without it, won't this result in php warnings in your viper_thread_comments_depth() function -- since it would then be declared as needing two args?

It seems to me that wp_parse_args-FUNCTION would then make more sense. But by doing so, there also is the problem of objects, which lands us to where we are with filter IDs (and more than the mere 5ms mentioned above).

Suggesting we close as wontfix and add the missing filters here and there as needed, personally... (Don't get me wrong, I like the idea too. But it somehow feels like a monster in the making if we proceed.)

[Viper007Bond]

It was a crazy idea that's evolving into something that maybe has potential, so yeah, lotta bad ideas at first. :P

The latest concept (that I mentioned above) is just adding a third (optional) parameter to the function called "context" (or something). You'd pass the function name (__FUNCTION__ is an easy way to do this) and then if that parameter is set, then a filter called wp_parse_args_the_context_value would be run. No context, no filter is run.

Using that setup (passing what function is using wp_parse_args()), you can do quite a bit. For example, here's the reason I came up with this concept in the first place:

​http://wordpress.pastebin.com/f58e26402

[Denis-de-Bernardy]

Right. But as I mentioned in my previous comment... if the idea evolves into a context that gets passed as a string, what difference does it make between:

• adding a filter to wp_parse_args, which would be called only when a context is provided (or always called with an empty string as context if none is supplied)
• adding the missing filter where doing the above would seem useful, and spare ourselves new mystifyingly abstract filters

Intuitively, I'd initially consider for the first option due to its elegance; and end up opting for second for simplicity and performance's sake.

[Viper007Bond]

As I said, the filter inside wp_parse_args() would only be run if a context was passed.

As for the second, take wp_list_comments() for example. Where would you put the filter? You'd add one that filtered $args or $r, or for simplicity, you'd just wrap the wp_parse_args() call in a filter:

$r = apply_filters( 'some_filter', wp_parse_args( $args, $defaults ), $args, $defaults );

Well what's the point of that when you could just have wp_parse_args() do the filter? Saves a lot of duplicated code.

[filosofo]

Replying to Viper007Bond:

Well what's the point of that when you could just have wp_parse_args() do the filter? Saves a lot of duplicated code.

Because otherwise you're going to influence performance geometrically. Let's say I want to change the max_depth argument. With a wp_list_comments-specific filter, my attached callback gets called once. With the wp_parse_args filter, it's called 80-some times.

Then let's say I want to do something similar elsewhere, e.g. in register_taxonomy(). With a register_taxonomy-specific filter, that callback fires once; with wp_parse_args, it's 80 times.

So with just 2 callbacks, we've turned what could be 2 function calls into 160.

And that's not even considering the issues making everything compatible with other plugins.

Note that I'm not necessarily arguing against this filter: I'm saying it should be a last resort choice when nothing else is available, like the "query" filter in wp-db. If we need more targeted filters, let's put those in first.

[Viper007Bond]

Replying to filosofo:

Because otherwise you're going to influence performance geometrically. Let's say I want to change the max_depth argument. With a wp_list_comments-specific filter, my attached callback gets called once. With the wp_parse_args filter, it's called 80-some times.

I can't edit the ticket and post the revised idea. You'll have to read the comments above. ;)

The current concept is adding a third optional parameter to wp_parse_args() that says where it's being called from (i.e. __FUNCTION__). If that parameter is set, then wp_parse_args() fires a filter called wp_parse_args_the_function_name.

So if I wanted to modify wp_list_comments(), I'd hook into wp_parse_args_wp_list_comments. My callback would only be run when wp_list_comments() is used rather than every time wp_parse_args() is used.

As for performance, there's no difference between adding a filter inside each function or just one dynamic filter inside of wp_parse_args() -- it's the same number of apply_filters(). Plus apply_filters() is incredibly fast, especially if nothing is hooked to the hook it's calling as all it's doing is looking at the hooks array and then aborting when it finds nothing for that hook. About a 100 calls to it with a callback only adds a couple milliseconds.

[filosofo]

Replying to Viper007Bond:

If that parameter is set, then wp_parse_args() fires a filter called wp_parse_args_the_function_name.

My bad. I thought you were talking about passing the function name as an argument.

As for performance, there's no difference between adding a filter inside each function or just one dynamic filter inside of wp_parse_args() -- it's the same number of apply_filters().

Well, not exactly. With the "wp_parse_args___FUNCTION__" filter, you're calling apply_filters to every single use of wp_parse_args, in plugin and core, whether it needs one or not.

About a 100 calls to it with a callback only adds a couple milliseconds.

I'm willing to believe your general point---that the performance impact is minimal---but it needs more testing. Those numbers don't mean much in isolation: what are we talking about, your multi-core dev environment? :)

[Viper007Bond]

Wrote a little test. I have a C2D E6750 (2.66GHz):

It took 0.3516919613 seconds to do 100000 apply_filters() calls to an unhooked filter, or 0.0000035169 seconds per call.

<?php

require( 'wp-load.php' );

$count = 100000;

$start = timer_stop( 0, 10 );

for ( $i = 1; $i <= $count; $i++ ) {
	apply_filters( 'this_is_a_fake_filter', false );
}

$stop = timer_stop( 0, 10 );
$diff = $stop - $start;
$per = number_format( $diff / $count, 10 );

echo 'It took ' . $diff . ' seconds to do ' . $count . ' apply_filters() calls to an unhooked filter, or ' . $per . ' seconds per call.';

?>

[Viper007Bond]

It seems to be 10 times slower the first time you call it (it must get smart and get faster starting the second time), but it's still not slow:

It took 0.0000350476 seconds to do 1 apply_filters() calls to an unhooked filter.

[dd32]

It seems to be 10 times slower the first time you call it (it must get smart and get faster starting the second time), but it's still not slow:

That'll be the overheads of the timer_*() functions.. That gets included in the entire calculation, so shared over thousands, its small, over 1 its more.. but close to nothing.

apply_filers() is pretty fast without a hooked function, as it short circuits the function early.

[westi]

I'm not a big fan of this.

I think that is is better to have targeted filters which are much safer for plugins to use.

If we add this filter every thing that is hooked on to it is going to have to check the context which is a really bad idea.

The more specifically targeted a filter the better.

If the filter was specific to the function being called that would be much more sensible - e.g. wp_parse_args_function_name

[markjaquith]

I agree that the more targeted wp_parse_args_function_name approach is better. And that would involve a lot of changes, so it's going to be too late for 2.9.

[Denis-de-Bernardy]

k... so in other words we add missing filters on a per need basis. Closing...

[westi]

Replying to Denis-de-Bernardy:

k... so in other words we add missing filters on a per need basis. Closing...

Please don't close tickets like this where there is clear interest from core devs is a solution to the request.

[Denis-de-Bernardy]

sorry... I was misinterpreting what the two of you had written. it seemed to me that the difference between:

$args = wp_parse_args($args, $defaults, 'foobar'); // applies wp_parse_args-foobar filter to the result

and:

$args = wp_parse_args($args, $defaults);
$args = apply_filters('foobar', $args);

went down to the second being less CPU intensive.

[Viper007Bond]

Replying to westi:

If the filter was specific to the function being called that would be much more sensible - e.g. wp_parse_args_function_name

Silly me. That makes a lot more sense. *facepalm*

[westi]

Are there specific places where people want to write a patch for this kind of filter?

Otherwise we should close this as maybelater.

[johnjamesjacoby]

Related/duplicate: #26673

[DrewAPicture]

Duplicate of #26673.