Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#12806 closed defect (bug) (fixed)

twentyten_setup can't be removed in a clean manner

Reported by: jorbin's profile jorbin Owned by:
Milestone: 3.0 Priority: high
Severity: normal Version: 3.0
Component: Themes Keywords: has-patch
Focuses: Cc:

Description (last modified by dd32)

Since child theme's functions.php files are loaded first, you can't use remove_action( 'after_setup_theme', 'twentyten_setup' ); to remove twentyten_setup. The attached patch reverts the change from [13886] and raps twentyten_setup in a functions_exists call to make it pluggable.

Attachments (1)

functions.diff (1.3 KB) - added by jorbin 14 years ago.

Download all attachments as: .zip

Change History (11)

14 years ago

#1 @sivel
14 years ago

  • Cc matt@… added

#2 @dd32
14 years ago

  • Description modified (diff)

#3 @rmccue
14 years ago

  • Cc me@… added

#4 @dd32
14 years ago

Given the reasoning for this patch, I cant see any reason why this function shouldn't be pluggable like the rest of the ones in the file.

Its a much cleaner solution than the hacky solution otherwise required.

#5 @dd32
14 years ago

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

(In [13923]) Restore pluggable for twentyten_setup(). Its impossible for child themes to override without using a callback to remove the filter at a higher priority. Props jorbin. Fixes #12806

#6 @nacin
14 years ago

I don't have a problem with [13923], but I fail to see how the example in the inline docs doesn't handle this without feeling like a hack:

 * Functions that are not pluggable (not wrapped in function_exists()) are instead attached
 * to a filter or action hook. The hook can be removed by using remove_action() or
 * remove_filter() and you can attach your own function to the hook.
 * In this example, since both hooks are attached using the default priority (10), the first
 * one attached (which would be the child theme) will run. We can remove the parent theme's
 * hook only after it is attached, which means we need to wait until setting up the child theme:
 * <code>
 * add_action( 'after_setup_theme', 'my_child_theme_setup' );
 * function my_child_theme_setup() {
 *     // We are replacing twentyten_setup() with my_child_theme_setup()
 *     remove_action( 'after_setup_theme', 'twentyten_setup' );
 *     // We are providing our own filter for excerpt_length (or using the unfiltered value)
 *     remove_filter( 'excerpt_length', 'twentyten_excerpt_length' );
 *     ...
 * }
 * </code>

#7 @rmccue
14 years ago

I'm fairly certain that is inaccurate though, as you need a higher priority than default.

#8 @jorbin
14 years ago

rmccue is correct, it doesn't work on the default priority and getting into action priorities might be getting a bit harder then is needed if we are going to be pushing child themes. I'm all for education, but we should also make the first step into coding WordPress (which is building a theme / child theme) as simple as possible.

#9 @nacin
14 years ago

Fair enough. How much else do we revert? I'm not really liking the idea of having callbacks for filters like excerpt_length being plugged. The filter should simply be removed. That's really what drove the earlier changes.

#10 @jorbin
14 years ago

I think the rest are fine because they can be added to whatever is hooked to after_setup_theme . I wouldn't complain if they became pluggable, but I don't think the need is there like it was with this one.

Note: See TracTickets for help on using tickets.