WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 6 weeks ago

#13239 reopened enhancement

Filter locate_template template_names variable

Reported by: chrisbliss18 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

I recently encountered a situation where it would be very helpful to supply alternate template file locations; however, this cannot be accomplished as the locate_template function is being used and that function's arguments are not filterable. So, I created a patch that adds the filter.

This patch adds two filters: locate_template and locate_template-TEMPLATENAME. This allows for both general and specific filtering.

The following example shows how this could be used to modify the location of a BuddyPress template file.

function filter_member_header_template( $template ) { 
    return dirname( __FILE__ ) . '/buddypress/members/single/member-header.php';
}
add_filter( 'locate_template-members/single/member-header.php', 'filter_member_header_template' );

While the value of this example is debatable as BuddyPress could be updated to support alternate template locations, the value of the patch itself is high. This opens up a new ability for plugins to modify template file locations, giving plugins a hook into the content rendering process without requiring themes to be modified.

Attachments (14)

13239.diff (921 bytes) - added by chrisbliss18 4 years ago.
Add filters to locate_template
13239.c2c.diff (852 bytes) - added by coffee2code 4 years ago.
13239.c2c.2.diff (1.9 KB) - added by coffee2code 4 years ago.
Amended version of my previous patch w/ extra hook added
13239.c2c.3.diff (1.3 KB) - added by coffee2code 3 years ago.
Only one filter added, but can do everything, including templates outside theme's directory
13239.c2c.4.diff (1.3 KB) - added by coffee2code 3 years ago.
Use path_is_absolute() to check for absolute paths
13239.5.diff (371 bytes) - added by wpmuguru 3 years ago.
13239.6.diff (1.3 KB) - added by coffee2code 21 months ago.
Refreshed my latest patch to cleanly apply against trunk.
13239.7.patch (360 bytes) - added by meloniq 13 months ago.
#22355 (template stacks) sounds better to me, but before it will get included into WP (and if even will), I would suggest adding simple filter in locate_template() to make possible to hook into this function with 'custom implementations of template stack'…
13239.2.diff (1.6 KB) - added by DrewAPicture 12 months ago.
24944.diff (617 bytes) - added by georgestephanis 8 months ago.
An alternate method.
13239.duckpunch.diff (1.2 KB) - added by georgestephanis 8 months ago.
13239.3.diff (2.1 KB) - added by DrewAPicture 2 months ago.
Hook docs + pre_locate_template
13239.8.diff (2.3 KB) - added by MikeSchinkel 2 months ago.
Based on 13239.2, adds hook docs, follows new brace standard, captures/uses $filepath and adds short-circuit.
13239.8a.diff (2.3 KB) - added by MikeSchinkel 2 months ago.
Fixed typo in 13239.8.diff

Download all attachments as: .zip

Change History (89)

chrisbliss184 years ago

Add filters to locate_template

comment:1 follow-up: nacin4 years ago

  • Keywords commit tested removed

I have spoken about this extensively with apeatling and jjj. We may be able to add a specific, targeted hook, or introduce a function or argument that exposes filters. But, I do not like the idea of "generic" filters that will run over and over again, especially against an area of the API that has so many file_exists checks and filters already running, so you can do nearly anything you want.

comment:2 nacin4 years ago

I understand you want to use this within the theme. We now have get_template_part() for that, so I'm not really sure what the use case might be that calls for one over the other.

comment:3 coffee2code4 years ago

I intended to write a ticket very similar to this one, so I'm glad to see it's already here.

There are at least three benefits to having such hooks:

  • Ability to alias a template file with another :
    • Statically (make requests for '404.php' use 'error.php')
    • Dynamically (i.e. if an admin is requesting 'searchform.php' serve them 'advanced-searchform.php')
  • Ability to hide a template : having locate_template() return "" would effectively make it seem as though the template doesn't exist
  • Ability to skip use of a template or certain type of template (i.e. prevent WP from searching for page-*, single-*, etc if you know you'll never use them or never want them used)

The attached patch, 13239.c2c.diff:

  • Refreshes the original patch
  • Adds a third filter, one that operates before the function returns, sending the template ultimately located
  • Passes $load and $require_once along to all filters to provide additional context
  • Removes extra file_exists() call

@nacin: I agree with your concern about the file_exists() call that the original patch is adding to every template searched for by locate_template(). My patch omits that part, so there is no overhead being added, other than calling some additional filters, which we're generally cool with.

My suggestion for @chrisbliss18, if my patch is accepted, would be to hook "locate_template-$template_name" and return "", which will cause the foreach to continue. Then separately hook "locate_template_located" and see if $template_names contains the template you want to override (the same template you hooked via "locate_template-$template_name"). If so, you can perform your own file_exists()and what have you and return your desired target template. You should be able to accomplish this without firing any unnecessary file_exists().

(Just as a related note: I ran across the issue while working on my Disable Search plugin. get_search_form(), the recommended and widely used method for themes to display the site search form, calls: locate_template(array('searchform.php')) and if it's found, requires it and immediately returns. Without one of the hooks in the attached patch, there is no way to suppress display of the search form without removing or renaming the template file. One could argue that this is ticket-worthy itself, but it's an example of something that could be improved via this ticket.)

The hooks for locate_template() allow overriding of core template-location functionality. Use of one of the filters could actually reduce the file_exists() checks if template(s) are being removed from being searched for since the hooks operate before any file_exists() are performed.

Some tangential relation to what scribu is doing #14310, but this applies closer to the metal and has more general applicability.

coffee2code4 years ago

comment:4 coffee2code4 years ago

In thinking about this a bit more, I do like what @chrisbliss18 what trying to achieve. While I suggested a way to achieve it using my previous patch, it could be facilitated more directly with an amended patch, 13239.c2c.2.diff. This patch incorporates my previous patch and introduces an additional hook, "locate_template_path-$template_name" which fires on a per-template_name basis.

Like the intent of @chrisbliss18's original patch, this allows for a custom override location to be defined for a given template. However, unlike the original patch this does not automatically trigger a file_exists() to achieve this. Instead, it sends the filter an empty string as the default value. Only if the hook actually returns a path will a file_exists() be attempted.

Patch also adds PHPDocs about the filters.

coffee2code4 years ago

Amended version of my previous patch w/ extra hook added

comment:5 idealien4 years ago

  • Cc idealien added

Was going to submit a similar enhancement but will merely add the use case I was thinking of for +1 validation of the potential for this.

A plugin creates custom post type(s) and has output that currently can be handled by shortcodes / widget and css modifications. Having the ability to register a template part would make for a more consistent implementation at the theme level as well as giving reference code for those customization situations were CSS alone is insufficient.

comment:6 nacin3 years ago

  • Milestone changed from Awaiting Review to 3.1

This looks good, but there are a lot of new filters and bits there. Do we really need all three?

(N.B. This can be punted in a week without a commit.)

comment:7 coffee2code3 years ago

After some consideration, I believe that the locate_template filter is the only absolutely necessary filter; two of the other three provide some specificity for hooking. I believe everything can be done with just that one. While the others may be nice to have, we'll still be fully served with just adding the locate_template filter. My latest patch, 13239.c2c.3.diff eliminates all but that filter.

However, there is one thing that can't be done simply with the addition of that filter: allowing templates to be located outside of the current theme's (or its parent's) directory. This is the functionality @chrisbliss18 was ultimately requesting. The latest patch addresses this by checking to see if an absolute path was provided as part of the template name. If so, then the template is checked first without the prepending of STYLESHEETPATH or TEMPLATEPATH.

Here's a simple plugin (tested) using the filter to allow a plugin to override theme templates with templates contained in the plugin's templates/ subdirectory:

/*
Plugin Name: Plugin that can override theme templates
Version: 0.1
Author: Mr. WordPress
Description: Templates in this plugin's templates subdirectory will override the same-named template from the theme.
*/

if ( !function_exists( 'add_plugin_template_path' ) ) :
/**
 * The plugin's templates/ subdirectory may contain overrides for any theme template file
 */
function add_plugin_template_path( $template_names ) {
	$new_template_names = array();
	foreach ( (array) $template_names as $tn ) {
		$new_template_names[] = dirname( __FILE__ ) . '/templates/' . $tn;
		$new_template_names[] = $tn;
	}
	return $new_template_names;
}
endif;
add_filter( 'locate_template', 'add_plugin_template_path' );

The case for the other filters can be made at a later date if they end up being desired.

coffee2code3 years ago

Only one filter added, but can do everything, including templates outside theme's directory

comment:8 nacin3 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

comment:9 scribu3 years ago

  • Cc scribu added

comment:10 DJPaul3 years ago

  • Cc DJPaul added

comment:11 sorich873 years ago

  • Cc sorich87@… added

comment:12 scribu3 years ago

Related: #12877

comment:13 andrewlawson3 years ago

I've also been looking for this feature. The request topic I submitted is at http://wordpress.org/support/topic/request-locate_template-directories-filter for anyone interested.

I also wrote a patch at https://gist.github.com/840853, although it's a little different to yours.

comment:14 chrisbliss183 years ago

@coffee2code,

Your last patch, 13239.c2c.3.diff, has a problem in its check for the absolute path. It assumes that any absolute path must begin with a forward slash. A better way to test for an absolute path is to use the path_is_absolute() function.

@andrewlawson,

I too considered filtering a set of search paths rather than the $template_names variable. I ended up not liking that idea as it is overly-restrictive to the way plugins and themes can modify behavior. For instance, the filter code must know the exact name of the template file it wishes to provide/replace and have that file available in the non-standard directory it has access to.

I can see this limiting at least two desirable abilities for code to use this filter:

  1. This would prevent code from providing an interface for users to select the templates they wish to use, requiring the code and template files to be custom-tailored for each specific application.
  2. It would make it impossible for code to override ID or slug-specific templates unless the filter code also had access to a full set of any possible ID or slug-specific templates.

My original intent for this patch was to open up a new ways for plugins to step in during the rendering process. Rather than limiting plugin's to having an all-or-nothing approach to modifying the front-end, they can have creative solutions that replace specific views or individual components (since control over locate_template() also gives control over get_template_part()), potentially introducing new ways for plugin and theme devs to collaborate to create powerful tools for users without resorting to proprietary implementations for this collaboration to occur.

Unfortunately, the patch you have produced guts most of that added value.

comment:15 andrewlawson3 years ago

  • Cc andrewlawson added

My patch wasn't to replace your's, I'd simply written it before I came across this ticket.

I can see the benefit of filtering $template_name instead of a list of directories, although there could be a problem with your patch:

Windows systems don't use '/' at the beginning of absolute paths, so

if ( $template_name{0} == '/' && file_exists( $template_name ) ) {

on line 1084 won't work. The only way to get around that is to just do

if ( file_exists( $template_name ) ) {

but that may cause problems for non-absolute paths depending on where the filter callback is defined.

I know windows isn't ideal for development, never mind production, yet it's still used for both, and needs to be accommodated for.

The only solution I can see is to use two filters; one to filter a list of directories, and another to filter the template names.

Version 1, edited 3 years ago by andrewlawson (previous) (next) (diff)

comment:16 scribu3 years ago

  • Keywords 3.3-early added; 3.2-early removed

comment:17 coffee2code3 years ago

At @chrisbliss18's suggestion, using the more robust path_is_absolute() to check for an absolute path in 13239.c2c.4.diff. That should address the concern of absolute path detection under Windows as well.

(On a related note, I proposed #17754 to improve path_is_absolute() since it could see more use with this patch applied.)

coffee2code3 years ago

Use path_is_absolute() to check for absolute paths

comment:18 wpmuguru3 years ago

I propose that the filter be moved to after the existing template search is complete.

The use case is loading templates contained within plugins. If it's moved to after then a plugin can contain logic to load either a template in the theme or one within the plugin. For example, a plugin can have a default template but the site owner can create a custom version of it and store the custom version in their theme.

wpmuguru3 years ago

comment:19 coffee2code3 years ago

I agree there is benefit to having a filter after the template search is complete. I proposed it as part of the attached patch 13239.c2c.2.diff. Since @nacin wasn't so sure about as many filters as I originally proposed, I ultimately decided 13239.c2c.4.diff was most robust.

One capability lost with the filter solely located after the template search is that you can't preempt the template search. Something I'm keen to do is prevent WP from having to do file_exists() checks for certain template formats that I know will never exist (such as all the $id based templates -- category-15.php, post-245.php, etc -- that I never use).

I'd be on board with also having the post-template-search filter that you favor (which I originally called 'located_template_located', but I don't mind whatever name it gets), but I'd like not to lose the pre-template-search filter.

comment:20 coffee2code3 years ago

Additionally, the post-template-search filter doesn't allow you to re-prioritize template preference search order, exclude certain templates from being located even if present, give preference to parent templates over child templates (maybe you don't want child themes to override a particular template), among other things.

You may be able to do some of that with the post-template-search filter, but in many instances you'd have to perform additional (and likely duplicate) file_exists() checks to determine if the already located template should be replaced with something else under current conditions.

To address the use case you mention: A plugin could make use of the pre-template-search filter to give preference to child and parent theme templates over itself, allowing the plugin's template to be the last resort if the theme doesn't otherwise provide an override.

comment:21 scribu3 years ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Reading through this discussion again, the filter is exactly what I proposed in #14310, except the patch on #14310 provides more context.

As such, closing as duplicate.

Last edited 3 years ago by scribu (previous) (diff)

comment:22 scribu3 years ago

On second though, given the history of that ticket, I should have closed that one as dup. If anyone else thinks so too, feel free to make the switch.

comment:23 coffee2code3 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

There may be some overlap, but I still think this has merit on its own. I acknowledged #14310 in my initial comment here. I still contend this solution is more robust.

One drawback in what appears to be the most current patch for #14310 is that it requires that the template type be specified to hook the filter.. i.e. add_filter( 'author_template_hierarchy', ... ). If I'm interested in processing all template location requests, I'd have to register hooks against all possible types.

That aside, #14310 operates at a higher level. I want lower level access via locate_template(). Not all template requests go through get_query_template(), but if you care about templates in the child OR parent theme, then locate_template() will be called. For instance, get_search_form() uses locate_template() to find searchform.php. Being able to short-circuit that is part of the impetus for my patch. #14310 won't let me intercept location of that template file.

I also think themes and plugins are more likely to call locate_template() to find specialized templates than to call get_query_template(). Even if they aren't, if the hook is in locate_template() you'll be able to intercede regardless of which of two functions are used.

I won't go so far as to close #14310 as a dup of this. I think what it hopes to accomplish could be done with my patch for this ticket, but its specificity might make things slightly easier in certain situations (i.e. you want to modify the template search order for just the 'author' templates). However, I'd like for my proposed patch for this to continue on for further consideration.

comment:24 coffee2code3 years ago

Marked #18151 as a duplicate.

comment:25 SergeyBiryukov3 years ago

  • Milestone set to Future Release

comment:26 husobj2 years ago

  • Cc ben@… added

I would also second a filter to preempt the template search as per coffee2code's comments above.

comment:27 in reply to: ↑ 1 mikeschinkel2 years ago

  • Cc mikeschinkel@… added

Replying to nacin:

I have spoken about this extensively with apeatling and jjj. We may be able to add a specific, targeted hook, or introduce a function or argument that exposes filters. But, I do not like the idea of "generic" filters that will run over and over again, especially against an area of the API that has so many file_exists checks and filters already running, so you can do nearly anything you want.

It may be moot by now in that this idea might be accepted, but we are running into situations where we really need for hook functionality in load_template(), so +1 to adding more hooks here.

comment:28 shawnkhall2 years ago

The way around unnecessary hooks then is to implement only the portion from the c2c.2.diff above:

$template_name = apply_filters( "locate_template-$template_name", $template_name, $load, $require_once ); 

Put that right inside the template_names loop and it'll prevent unnecessary execution for invalid templates. This one change would address the problems identified by most within this thread and others (#16541).

comment:29 anmari2 years ago

  • Cc anmari@… added

comment:30 scribu22 months ago

A more low-level filter proposal for load_template(): #21062

coffee2code21 months ago

Refreshed my latest patch to cleanly apply against trunk.

comment:31 coffee2code21 months ago

Attached 13239.6.diff as a refresh of my latest patch so that it applies cleanly against trunk as of r21446.

comment:32 iandunn20 months ago

  • Cc ian_dunn@… added

comment:33 DeanMarkTaylor16 months ago

  • Cc DeanMarkTaylor added

comment:34 iandunn15 months ago

Any chance this could be considered for 3.6?

Call me anal-retentive, but I want my theme directory to look like this, not like this.

comment:35 rmccue15 months ago

Considering this was marked 3.2-early, and then 3.3-early, this sounds like it should be in by now.

comment:36 wonderboymusic15 months ago

FYI: yesterday, there were 76 open tickets still tagged 3.2-early. I have been going through them - anyone that wants to help, have at it.

comment:37 rmccue15 months ago

  • Milestone changed from Future Release to 3.6

Bumping to 3.6 as per wonderboymusic's triaging. Patch still applies cleanly to trunk.

comment:38 iamfriendly15 months ago

  • Cc richard@… added

comment:39 fjarrett13 months ago

  • Cc fjarrett@… added

meloniq13 months ago

#22355 (template stacks) sounds better to me, but before it will get included into WP (and if even will), I would suggest adding simple filter in locate_template() to make possible to hook into this function with 'custom implementations of template stack'...

comment:40 meloniq13 months ago

  • Cc meloniq@… added

comment:41 meloniq13 months ago

As in comment of my above patch:
#22355 (template stacks) sounds better to me, but before it will get included into WP (and if even will), I would suggest adding simple filter in locate_template() to make possible to hook into this function with 'custom implementations of template stack'...

comment:42 coffee2code13 months ago

@meloniq: The comment associated with your patch doesn't justify your reasoning for placing the filter where you've put it in the function. If you've seen all the other patches to the ticket, there is no dispute about having a filter in locate_template() since that's what they're all doing.

Your patch places the filter after the file_exists() checks, which means that when the filter is used, a number of file_exists() checks will have been performed unnecessarily. It also assumes that the file specified via the filter exists since it won't go through the check itself. You pretty much lose the template hierarchy fallback approach employed in locate_template(), as well as the ability to easily locate a custom template in either a parent or child theme.

comment:43 meloniq13 months ago

@coffee2code, sorry, just noticed that my patch is duplicate of patch submitted by @wpmuguru - I could leave just an comment... As I said in my previous comment, I like more the idea with 'template stacks' ( see #22355 ) where You can simply register new template location with priority, so plugins like WPEC, WC or BP could register template location with lower priority, and included template will be loaded from this location only if theme or child theme does not have it...

Suggested filter after the parent/child checks seems to be safe, does not require extra testing (3.6 is close), and gives full control over this what happening in locate_template()... so before we will find perfect solution to get out from this limited hierarchy (child->parent), with which everyone would agree that is the way to go - this filter would allow for it (load templates from other locations).

Note: file_exists() doesn't seems to be an expensive function, and it have cache: http://php.net/manual/en/function.file-exists.php

DrewAPicture12 months ago

comment:44 DrewAPicture12 months ago

13239.2.diff refreshes @coffee2code's 13239.6.patch. I also swapped out STYLESHEETPATH and TEMPLATEPATH for get_stylesheet_directory() and get_template_directory() respectively.

The filter introduced with 13239.2.diff works as expected.

Would be nice to get this in for 3.6, even if we end up taking a more far-reaching approach in the future in #22355.

comment:45 ocean9010 months ago

  • Milestone changed from 3.6 to Future Release

comment:46 georgestephanis8 months ago

#24944 was marked as a duplicate.

georgestephanis8 months ago

An alternate method.

comment:47 follow-up: georgestephanis8 months ago

Just added 24944.diff as an alternate way -- just letting things choose to duck punch what file to use. Less complex, possibly less elegant.

comment:48 in reply to: ↑ 47 meloniq8 months ago

Replying to georgestephanis:

Just added 24944.diff as an alternate way -- just letting things choose to duck punch what file to use. Less complex, possibly less elegant.

Patch '24944' does not seems to respect value of $load variable, do not loads the template if true is passed.

comment:49 georgestephanis8 months ago

Good call, I'll have a new one up shortly.

comment:50 georgestephanis8 months ago

meloniq: new patch included that just skips the population of $located if the filter returns something.

comment:51 talbet8 months ago

  • Cc talbet.fulthorpe@… added

comment:52 ircbot2 months ago

This ticket was mentioned in IRC in #wordpress-dev by rmccue. View the logs.

comment:53 rmccue2 months ago

  • Keywords commit added

I need more duck-punching in my life!

On a serious note, given this was 3.2-early, then 3.3-early, and we're in 3.9 territory now (*and* I need this filter), marking for commit. Patch looks good, assuming we're happy with the name. (pre_locate_template might better match the existing style, ala pre_get_option)

comment:54 georgestephanis2 months ago

Plz plz plz plz plz to haz

comment:55 SergeyBiryukov2 months ago

  • Keywords needs-docs added
  • Milestone changed from Future Release to 3.9

DrewAPicture2 months ago

Hook docs + pre_locate_template

comment:56 DrewAPicture2 months ago

  • Keywords needs-docs removed

13239.3.diff adds hook docs and renames the filter hook to pre_locate_template for naming consistency.

comment:57 follow-ups: coffee2code2 months ago

I still prefer @DrewAPicture's update in 13239.2.diff of my most recent patch. It permits customization of the template search stack (via the filter) and allows for templates to be located outside of the theme or its parent theme directories while still retaining the benefit of WP's file existence checks, its hierarchical template search, and adds the use of get_stylesheet_directory() and get_template_directory() rather than constants.

I'm not a fan of bailing on everything locate_template() does if the filter as proposed by @georgestephanis returns a specific template to load. You lose all the locating functionality of the function, forcing any hooking code to likely duplicate that to ensure the template exists. More than anything, though, I have concerns it makes it harder to play well with others; it'll become a problem if multiple functions hook the filter since the first invoked callback may have chosen a template to load, leaving subsequent callbacks unable to determine how the template was chosen and unable to influence the template selection process (short of ignoring any previous filtered selection and hoping a later callback doesn't override its own selection).

comment:58 in reply to: ↑ 57 MikeSchinkel2 months ago

Replying to coffee2code:

I still prefer @DrewAPicture's update in 13239.2.diff of my most recent patch.
I'm not a fan of bailing on everything locate_template() does if the filter as proposed by @georgestephanis returns a specific template to load. You lose all the locating functionality of the function, forcing any hooking code to likely duplicate that to ensure the template exists. More than anything, though, I have concerns …

Very much agree.

BTW, we use our own version of locate_template() so that we can implement "skins" which is similar in concept to child themes but for specific sections of a site and all contained within the same theme. For example, the /about/ section could have a red-ish skin, the /products/ section a green-ish skin and so on (it's more complex than that, but hopefully you get the idea.)

It would be nice to be able to implement things like skins without having to bypass standard WordPress API functions, in this case locate_template().

adds the use of get_stylesheet_directory() and get_template_directory() rather than constants.

I would suggest not calling those functions twice but instead capturing the return value the first time called. Is there a good reason not to capture to a variable instead of calling at least one of them twice?

comment:59 in reply to: ↑ 57 rmccue2 months ago

Replying to coffee2code:

More than anything, though, I have concerns it makes it harder to play well with others; it'll become a problem if multiple functions hook the filter since the first invoked callback may have chosen a template to load, leaving subsequent callbacks unable to determine how the template was chosen and unable to influence the template selection process (short of ignoring any previous filtered selection and hoping a later callback doesn't override its own selection).

You've convinced me on this point alone. I preferred the other patch, but after considering it, the array filter is more flexible.

+1 on either patch, but preference to 13239.2.diff.

pls can has?

comment:60 follow-up: rmccue2 months ago

Also worth mentioning: I don't think we should pass $load and $require_once to the filter.

comment:61 in reply to: ↑ 60 ; follow-up: DrewAPicture2 months ago

Replying to rmccue:

Also worth mentioning: I don't think we should pass $load and $require_once to the filter.

I agree if if we end up going with 13239.2.diff (plus hook docs). Those booleans are far less important if all we're doing is filtering the template names.

MikeSchinkel2 months ago

Based on 13239.2, adds hook docs, follows new brace standard, captures/uses $filepath and adds short-circuit.

comment:62 in reply to: ↑ 61 ; follow-up: MikeSchinkel2 months ago

Replying to DrewAPicture:

I agree if if we end up going with 13239.2.diff (plus hook docs).

I just added 13239.8.diff based on 13239.2.diff with the following changes:

  1. Adds hook docs.
  2. Follows new bracing coding standard (i.e. don't use single line if() without braces.)
  3. Added spaces in expressions to be consistent with WordPress coding standard
  4. Added ability to short-circuit loading a template file by having the filter return false.
  5. Captured value of expressions that called get_stylesheet_directory() and get_template_directory() into variable $filepath so those functions need not be called twice (and so that value is easier to debug within locate_template()).

comment:63 in reply to: ↑ 62 ; follow-up: rmccue2 months ago

Replying to MikeSchinkel:

  1. Captured value of expressions that called get_stylesheet_directory() and get_template_directory() into variable $filepath so those functions need not be called twice (and so that value is easier to debug within locate_template()).

I thought you meant something more like:

$stylesheet = get_stylesheet_directory();

// ...
} else if ( file_exists( $stylesheet . '/' . $template_name ) ) { 
         $located = $stylesheet . '/' . $template_name; 
         break;

(Ditto for template directory.)

The code currently calls get_stylesheet_directory() and get_template_directory() once per iteration.

(The else if ( $filepath = file_exists( $filepath = get_template_directory() . '/' . $template_name ) ) line also means $filepath ends up being true rather than the path. :) )

comment:64 in reply to: ↑ 63 MikeSchinkel2 months ago

Replying to rmccue:

I thought you meant something more like: ...

Why would you want to evaluate the expression $stylesheet . '/' . $template_name twice?

The code currently calls get_stylesheet_directory() and get_template_directory() once per iteration.

Not for the one the matches, it's called twice.

(The else if ( $filepath = file_exists( $filepath = get_template_directory() . '/' . $template_name ) ) line also means $filepath ends up being true rather than the path. :) )

Doh! Stupid typo (so hard (for me) to get these right the first time. It's not for lack of trying.)

13239.8a.diff will come next.

MikeSchinkel2 months ago

Fixed typo in 13239.8.diff

comment:65 follow-up: georgestephanis2 months ago

Just a bit of clarification, the reason I had included the option to fully duck-punch the functionality -- and pass in the $load and $require_once was so that it could -- for example -- clock analytics on how long each template include takes to run on average, and cache ones as determined by the back-end.

The ability to add an absolute path and have it included is spiffy for the latter part, but there's no good way to clock the time it takes for the include to evaluate, short of faking a duck-punch with the existing filter and snagging the arguments passed in via a debug_backtrace()

If that's something that we don't wanna support, that's fine, just wanted to drop in an explanation of why it had been there.

comment:66 in reply to: ↑ 65 MikeSchinkel2 months ago

Replying to georgestephanis:

If that's something that we don't wanna support, that's fine, just wanted to drop in an explanation of why it had been there.

I only removed those in the patch I uploaded because others suggested they were not needed. I personally have not strong opinion on that topic either way.

comment:67 follow-up: obenland2 months ago

  • Keywords 3.3-early commit removed

How about casting $template_names to an array within the filter on line 475?

We could omit the false === $template_names check and remove the typecasting from the foreach loop, while not changing the outcome for the function. Added benefit would be a reduced type ambiguity, plugins could always expect an array.

comment:68 in reply to: ↑ 67 MikeSchinkel2 months ago

Replying to obenland:

How about casting $template_names to an array within the filter on line 475?

Funny, that was how I wrote it initially. I reverted because I didn't want the filter to have to check for a one element array with no value which is what would happen if a false value or an empty string value was passed (which admittedly would be a very rare edge case.)

We could omit the false === $template_names check and remove the typecasting from the foreach loop, while not changing the outcome for the function.

Actually the purpose of the false === $template_names was to allow the filter to return false if the hook wants to short-circuit template loading. So omitting would change the outcome in some cases. I myself have the same need in a library I've written where I want to run my own load_template() because I want to be able to add a $view variable into the template scope.

(Another option way to solve my specific use-case besides short-circuiting would be to add a filter in load_template() that would get passed the name of global variables to add to the scope and then let the hook add or remove them as needed. The return value would then use an extract() on the array returned. But I don't know what other use-cases for short-circuiting might exist so I don't know if my use-case is the only one.)

Last edited 2 months ago by MikeSchinkel (previous) (diff)

comment:69 ircbot6 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:70 casben796 weeks ago

Would it be possible to add a filter to the full path right before its passed back?

Here is the use case I have.

I have a plugin which supports templating of small parts of output (http://wordpress.org/plugins/wp-slick-slider/), it currently has some rather messy code like below:

if( file_exists( $wpss_templatepath . '/' . $slider . '_full-width-image.php' ) ){
    $include = $wpss_templatepath . '/' . $slider . '_full-width-image.php';
} elseif( file_exists( $wpss_templatepath . '/full-width-image.php' ) ) {
    $include = $wpss_templatepath . '/full-width-image.php';
} else {
    $include = $wpss_default_path . '/full-width-image.php';
}

if I clean that up to something like this:

$templates = array(
    "wpss/{$slider}_full-width-image.php",
    "wpss/full-width-image.php",
);
$include = locate_template( $templates );

if( $include === '' ){
    $include = $wpss_default_path . '/full-width-image.php';
}

and there is a filter on the full path in locate_template() like so:

return apply_filters( 'after_locate_template', $located, $template_names );

This would allow my plugin (and any other plugin whose author uses locate_template() to template the output) to be modified by another plugin of mine (http://wordpress.org/plugins/wp-template-overrides/) which is designed to tweak template files by creating a file in a completely different path (/wp-content/theme-overrides/) and hooking into template_include

Using this combination, any user would be able override any templating that uses locate_template() without modifying the theme (EG: premium themes)

comment:71 follow-up: nacin6 weeks ago

  • Milestone changed from 3.9 to Future Release

I still have mixed feelings about this filter. As I mentioned in IRC, the a root question is whether the template hierarchy should be something you can butcher.

As I've also argued previously (elsewhere, apparently), a huge benefit of the template hierarchy in WordPress is that it's predictable. You can open up any theme and understand how it works. It's one of the most important and easy-to-understand concepts in WordPress.

That said, rmccue has been kicking around an alternative idea that would allow one to filter the directories in which a file is searched. See #27322 . Now that's interesting, because it simply adds a layer on top of the parent/child relationship, versus munging the well-established template hierarchies. Between that and the template_include filter, and the existing other filters (such as in get_query_template()), should be a significant amount of power.

comment:72 casben796 weeks ago

Im not at all suggesting messing with the template hierarchy, but being able to programmatically control the location of small subsets of the code, locate_template() is also used in get_template_part() which really has nothing to do with the overall template hierarchy.

The overall hierarchy is already very easy to butcher at the moment using the template_include filter, which is at the moment the only place my plugin can effect the changes people make to the templates.

comment:73 in reply to: ↑ 71 ; follow-up: MikeSchinkel6 weeks ago

Replying to nacin:

As I've also argued previously (elsewhere, apparently), a huge benefit of the template hierarchy in WordPress is that it's predictable. You can open up any theme and understand how it works. It's one of the most important and easy-to-understand concepts in WordPress.

Gotta agree with this. Standardization is super important.

That said, rmccue has been kicking around an alternative idea that would allow one to filter the directories in which a file is searched. See #27322 .

Yes, this is really nice and would address the 'skins' use-case I mentioned above.

Replying to casben79:

locate_template() is also used in get_template_part() which really has nothing to do with the overall template hierarchy.

Gotta agree with that, too, because that's where some of the biggest limitations come from. And also get_header() and get_footer().

The overall hierarchy is already very easy to butcher at the moment using the template_include filter, which is at the moment the only place my plugin can effect the changes people make to the templates.

And that it true too. Currently if people want to butcher it they can. They just do it in less standardized ways.

So…I think casben79 identified the biggest pain point, the template parts (and the headers and footers.) Maybe we could enable them to be changed? I've had to bypass get_template_part() in some of my projects because the simple two level hierarchy stored in the theme root doesn't allow for maintainable template parts.

Or, maybe it's time to consider adding more robust template parts hierarchy to address complex use-cases?

comment:74 in reply to: ↑ 73 ; follow-up: rmccue6 weeks ago

Replying to MikeSchinkel:

Or, maybe it's time to consider adding more robust template parts hierarchy to address complex use-cases?

IMO, more complex cases like this are best handled by the theme itself. There's nothing stopping you adding a mytheme_get_template_part that handles any sort of use cases you need, backed by locate_template in the same way that the normal get_template_part is. Introducing more complicated part hierarchy into core removes the flexibility of doing it yourself.

comment:75 in reply to: ↑ 74 MikeSchinkel6 weeks ago

Replying to rmccue:

IMO, more complex cases like this are best handled by the theme itself. There's nothing stopping you adding a mytheme_get_template_part that handles any sort of use cases you need, backed by locate_template in the same way that the normal get_template_part is.

But then you have a theme that nobody except the developer of the theme and its (small group of) loyal users understand.

Introducing more complicated part hierarchy into core removes the flexibility of doing it yourself.

How does adding flexibility to enable get_template_part() to support a new but defined structure removes flexibility? If a structure for template parts were added to core you'd still be able to do exactly what you suggested if you don't like the new structure.

That said, doesn't the flexibility you speak of contradict @nacin's assertion that the predicability of the template hierarchy (and thus of a potential template parts hierarchy) is a plus?

Note: See TracTickets for help on using tickets.