WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#12748 closed defect (bug) (fixed)

Rethink some of the pluggable architecture of twentyten functions.php

Reported by: nacin Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Themes Keywords:
Focuses: Cc:

Description

This came out of #12745.

In functions.php, we allow twentyten_init() to be plugged by a child theme, yet it also is only a callback for a hook.

We should instead offer documentation and education for the child theme to remove that hook if they don't want twentyten_init() to be run. I'm not sure it needs to be pluggable when it is only an action hook callback.


Thinking further, I've seen various forward-thinking recommendations to use action hooks instead of custom template tags in plugins. So perhaps instead of twentyten_term_list(), we use do_action('twentyten_term_list') in themes instead.

Or, even better, we add filters we need in the taxonomy API, and use the_tags() and the_category(). When a theme needs to call $wp_query->get_queried_object() in this fashion, we should come up with ways to make it better.

See also:

wp_title() -- #12370 (smarter version) and #11951 (include paging), and twentyten_get_page_number() and twentyten_the_page_number() (the former is only used by the latter, also, and the latter is only used in header.php to construct a page title).

$wp_query->max_num_pages > 1 in loop.php.

Change History (8)

comment:1 @mikeschinkel5 years ago

  • Cc mikeschinkel@… added

+1

comment:2 follow-up: @nacin5 years ago

(In [13886]) Second pass on twentyten/functions.php inline documentation. see #12695. Also:

rename twentyten_init() to twentyten_setup(), to reflect the hook it is run on (init would be too late, it must be after_setup_theme). Remove unnecessary twentyten_get_page_number() and transfer functionality to twentyten_the_page_number(). Remove the function_exists() wrappers from functions that are simply tied to a hook, as a better practice would be to remove the hook instead of plugging the function. For architecture changes, see #12748.

comment:3 @nacin5 years ago

I just realized of course, that with the child theme being included first, then calling remote_action or remove_filter in the body of the child's functions.php would do nothing, as the actions would not be added yet.

So we should instruct them to remove them on the after_setup_theme hook, which would work as long as it is priority <= 10. I feel that is still better practice (and easier, I think) than telling them that the functions are pluggable.

Thoughts?

comment:4 in reply to: ↑ 2 @mikeschinkel5 years ago

Replying to nacin:

Remove unnecessary twentyten_get_page_number() and transfer functionality to twentyten_the_page_number().

Don't want to hijack the thread but I've recently been confused by all the inconsistency with the_*() and get_*() function in WordPress core. It seems, FWIW, combining them is going in the wrong direction as there is no consistent parameter positioning in which the_*() function indicate echo vs. return.

Is there some ticket where we can discuss this with a possible goal is coming up with consistency across all of WordPress (or should I create such a ticket?)

comment:5 @nacin5 years ago

Simple answer: Don't consider these functions to be part of the core, just a theme. :)

We're simply doing this because wp_title() isn't smart enough to handle page numbers (#12370, #11951). When we have one function that does the work -- including the <title> presentational aspect of adding " | Page: " -- and is unused except in the function that echoes the result, it's safe in my opinion to combine them. This isn't an API that will be used by others, just an internal helper function for a theme.

I'm tempted to propose a wp_title_paging() or something that echoes nothing if get_query_var('paged') < 2, but then kicks into action with a separator and text before/after if it is paged. But really, that should be tackled together with making a more powerful wp_title().

There are always tickets out there that attempt to create more consistency with get_* (often get_the_*) and the_*. Not sure what the status of each is though.

comment:6 @nacin5 years ago

(In [13888]) Clarify order of operations for a child theme removing a filter of a parent theme. see #12695, see #12748

comment:7 @nacin5 years ago

(In [13889]) Use $paged global instead of get_query_var('paged') in Twenty Ten. see #12748

comment:8 @nacin5 years ago

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

Other tickets can be used for additional cleanups.

Note: See TracTickets for help on using tickets.