Skip to content

Commit

Permalink
Fix broken deprecation
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke Carbis committed Apr 13, 2014
1 parent 831abdb commit 768cc07
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions includes/deprecated.php
Expand Up @@ -17,32 +17,40 @@
),
);

foreach ( $wp_stream_deprecated_filters as $old => $new ) {
add_filter( $old, 'wp_stream_deprecated_filter_mapping' );
foreach ( $wp_stream_deprecated_filters as $filter ) {
add_filter( $filter['new'], 'wp_stream_deprecated_filter_mapping' );
}

function wp_stream_deprecated_filter_mapping( $data ) {
global $wp_stream_deprecated_filters;

$filter = current_filter();
$current_filter = current_filter();
$deprecated_filter = false;

if ( ! has_filter( $filter ) ) {
foreach ( $wp_stream_deprecated_filters as $key => $filter ) {
if ( $current_filter === $filter['new'] ) {
$deprecated_filter = $key;
break;
}
}

This comment has been minimized.

Copy link
@lukecarbis

lukecarbis Apr 13, 2014

Contributor

This is why I originally organised the $wp_stream_deprecated_filters array with the new filter as the key (rather than the old filter as the key, as it is now). That way we wouldn't have to loop through to figure out what the name of the deprecated filter is. We would only have to refer to $wp_stream_deprecated_filters[ $current_filter ]['old'].

This comment has been minimized.

Copy link
@shadyvb

shadyvb Apr 14, 2014

Contributor

Please do mend it as you see appropriate, let me know when you're ready for merge.


if ( ! $deprecated_filter || ! has_filter( $deprecated_filter ) ) {
return $data;
}

$filter_args = array_merge(
array(
$wp_stream_deprecated_filters[ $filter ]['new'],
$deprecated_filter,
),
func_get_args()
);

$data = call_user_func_array( 'apply_filters', $filter_args );

_deprecated_function(
sprintf( __( 'The %s filter', 'stream' ), $filter ),
$wp_stream_deprecated_filters[ $filter ]['version'],
$wp_stream_deprecated_filters[ $filter ]['new']
sprintf( __( 'The %s filter', 'stream' ), $deprecated_filter ),
$wp_stream_deprecated_filters[ $deprecated_filter ]['version'],
$current_filter
);

return $data;
Expand Down

3 comments on commit 768cc07

@lukecarbis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shadyvb Here's the explanation of what's happening here:

Our goal here is to show a notice, and make the filter work (with all arguments) even though it's deprecated.

Firstly, all deprecated filters are looped through (ln 20). Nothing will happen if we add a filter to the deprecated filter (since it no longer gets called). Instead we add the wp_stream_deprecated_filter_mapping function to the deprecated filter's replacement.

Now inside the wp_stream_deprecated_filter_mapping we can easily know which filter is currently being called, but we have to loop through our list of deprecated filters (ln 30-35) to know which filter it replaces.

Once we've got that, we check to see if there is actually a result (there should be, otherwise we wouldn't be here in the first place) AND we check to see if someone somewhere somehow is calling the old deprecated filter. If nobody is trying to call the old deprecated filter, then we can stop right there.

If they are, then we want to apply the function they wanted to run on the old filter, to the data of this replacement filter. We want to run something like this:
$data = apply_filters( 'old_filter', $arg_1, $arg_2, $arg_∞ );

So we take the args that were passed to this filter function, and prepend them with the name of the deprecated filter (ln41-46). Now we can run call_user_func_array to get whatever value the old filter functions wanted to return (ln 48). Doing it this way lets us pass a dynamic amount of args.

Finally we display a notice.

@lukecarbis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's some code which you can add to the top of the file for testing:

add_filter( 'stream_toggle_filters', 'test_filter_toggle' );
function test_filter_toggle( $filters ) {
    $filters['test'] = __( 'Test', 'stream' );
    return $filters;
}

This should create a deprecated notice, as well as add a 'Test' filter inside Screen Options.

@shadyvb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukecarbis I guess I've got it wrong from the beginning :) Great work man 👍 /five!

Please sign in to comment.