Opened 23 months ago
Last modified 3 months ago
#18298 new enhancement
deprecate TEMPLATEPATH and STYLESHEETPATH — at Version 17
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Themes | Version: | 3.0 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | info@…, chip@…, me@…, pippin@…, omarabid, travis@…, juliobosk@… |
Description (last modified by aaroncampbell)
As part of #15086 we wanted to add unit tests, but we came across a problem in #UT22. Basically locate_template() uses these constants instead of get_template_directory() and get_stylesheet_directory() which makes it impossible to use switch_theme(). It looks like the constants are only used in about 20 places through core, so I recommend we fix those places and deprecate the constants.
Change History (20)
comment:2
lloydbudd
— 23 months ago
deprecate the constants
This would be very well received by the WordPress.com VIP team. We are regularly cleaning up TEMPLATEPATH and STYLESHEETPATH, replacing with the get_ functions in VIP partners themes' function to ensure features like mobile theme plugins and post-by-email work which don't go through initialization of those constants.
aaroncampbell
— 23 months ago
comment:4
aaroncampbell
— 23 months ago
- Keywords has-patch added; needs-patch removed
The patch removes all core uses of the constants. It's still used in twentyten and twentyeleven, so those should be fixed as well.
I'm not sure how we want to deprecate constants, so I just left them there. If we don't have any way in place to actually deprecate them, we should at LEAST add comments that say they're deprecated.
comment:5
scribu
— 23 months ago
Yeah, it seems the practice is simply to add a @deprecated tag. Example: PLUGINDIR
aaroncampbell
— 23 months ago
comment:6
aaroncampbell
— 23 months ago
18298.2.diff adds the @deprecated tag to the constants.
comment:7
cais
— 22 months ago
Following along the lines of the @since documentation would there be a general benefit to having the version when the item is deprecated included as well?
For example, from this ticket's premise, @deprecated 3.3
comment:8
chipbennett
— 22 months ago
- Cc chip@… added
Shouldn't these constants have version/replacement function information in the @deprecated phpDoc tag? (I ask mainly so that the various development tools, such as the Log Deprecated Notices Plugin and the Theme Check Plugin can be configured to pick up this information, to pass along to developers.
aaroncampbell
— 22 months ago
comment:9
aaroncampbell
— 22 months ago
18298.3.diff adds php doc like this:
* @deprecated 3.3 * @deprecated Use get_stylesheet_directory()
comment:10
follow-up:
↓ 11
scribu
— 22 months ago
@chip: Log Deprecated Notices plugin can't do anything about it, since there's no mechanism in PHP to find out when a particular constant is used. Same for global variables (which is why they're evil).
comment:11
in reply to:
↑ 10
chipbennett
— 22 months ago
Replying to scribu:
@chip: Log Deprecated Notices plugin can't do anything about it, since there's no mechanism in PHP to find out when a particular constant is used. Same for global variables (which is why they're evil).
Why is that? Couldn't either Plugin read the phpDoc headers, and cross reference the @deprecated tag against the constant being used anywhere in the Theme files? (Isn't that what they do anyway, to provide the deprecation (since-version, alternative function)?
comment:12
follow-ups:
↓ 13
↓ 14
aaroncampbell
— 22 months ago
I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used since there's no way to actually throw a deprecated notice since it's not a function. For example, how do you find:
$test = $prefix . TEMPLATEPATH . $suffix;
but not
$test = $prefix . 'TEMPLATEPATH' . $suffix;
$test = "$prefix TEMPLATEPATH $suffix";
$test = <<<TEST
$prefix
Bad: TEMPLATEPATH
$suffix
TEST;
comment:13
in reply to:
↑ 12
chipbennett
— 22 months ago
Replying to aaroncampbell:
I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used since there's no way to actually throw a deprecated notice since it's not a function.
Pross is the WPTRT's resident RegEx ninja; if anybody can figure out a way to do it, he can. :) So, maybe it's something that can find its way into Theme Check. My interest in the question (which now exceeds the scope of the patch itself) is in providing as much information as possible to Theme Developers, and getting them pre-emptively to begin to adopt the use of get_template_directory_*() and get_stylesheet_directory_*().
comment:14
in reply to:
↑ 12
;
follow-up:
↓ 15
toscho
— 22 months ago
Replying to aaroncampbell:
I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used
Impossible. PHP is more complex than PCRE, so you’d have to use a tokenizer. Just think about constants in comments … ;)
comment:15
in reply to:
↑ 14
chipbennett
— 22 months ago
Replying to toscho:
Replying to aaroncampbell:
I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used
Impossible. PHP is more complex than PCRE, so you’d have to use a tokenizer. Just think about constants in comments … ;)
Which would be just fine for Theme Check. It ignores anything in PHP comments. Also, even if it finds false-positives, that's okay. Most such output is listed as INFO anyway: meant as a hint to the developer/reviewer to verify that its usage is proper.
In any case, this rabbit trail has strayed far from the intent of the ticket. I'll take it offline with Pross, and if he comes up with something viable for Theme Check, then I will take a look at the rest of the defined constants with @deprecated, to see if any could benefit from additional deprecation information, and submit a separate patch.
comment:16
nacin
— 19 months ago
- Type changed from defect (bug) to enhancement
comment:17
aaroncampbell
— 19 months ago
- Description modified (diff)
- Version set to 3.0
20 places isn't exactly few, but I agree that functions are better than constants, in this case.