WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#11388 closed enhancement (fixed)

Deprecated functions: Formally deprecate and/or move to deprecated.php

Reported by: nacin Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9
Component: Inline Docs Keywords: has-patch commit
Focuses: Cc:

Description

A few functions that should be moved to deprecated.php, found when going through deprecated code and phpdoc in #7493 and #11386. Many are missing the _deprecated_function() call but are marked as deprecated in phpdoc. Some have the call and just need to be moved.

Move to deprecated.php and improve phpdoc, unless there is a reason to keep these in 110n.php --

__ngettext()
__ngettext_noop()

Move to deprecated.php, improve phpdoc, and add _deprecated_function() call --

get_the_attachment_link()
get_attachment_icon_src()
get_attachment_icon()
get_attachment_innerHTML()
get_link()
dropdown_categories()
dropdown_link_categories()

Additionally, dev-feedback is necessary for two situations:

  1. To consider the pre-2.8 escape functions in formatting.php as candidates for formal deprecation.
  1. To consider a new pluggable-deprecated.php (or deprecated-pluggable.php) file to hold pluggable functions that have since been deprecated, to discourage their use and lower their profile. They are wp_setcookie(), wp_clearcookie(), wp_get_cookie_login(), and wp_login(). Or, at the very least, wp_login() needs a _deprecated_function() call added.

Waiting to formally patch, pending traction and feedback on all of this.

Attachments (4)

11388.diff (32.3 KB) - added by nacin 6 years ago.
Formally deprecates 10 functions: get_attachment_innerHTML, get_attachment_icon, get_attachment_icon_src, get_the_attachment_link, ngettext_noop, ngettext, get_link, wp_dropdown_cats, dropdown_link_categories, dropdown_categories
11388.2.diff (36.3 KB) - added by nacin 6 years ago.
Second patch includes four pre-2.8 escape functions. These were already marked as deprecated in phpdoc.
11388.3.diff (36.8 KB) - added by nacin 6 years ago.
Third patch (these are culmulative) removes dropdown_link_categories from wp-admin/includes/template.php. It had been added to deprecated.php but not removed from its original position. All other functions double-checked.
11388.4.diff (37.9 KB) - added by nacin 5 years ago.
Deprecates translate_with_context() in favor of _x(). Commiting dev may wish to change reference to _x() to translate_with_gettext_context().

Download all attachments as: .zip

Change History (24)

comment:1 @scribu6 years ago

  • Keywords needs-patch added
  • Type changed from defect (bug) to enhancement

+1 on all proposals.

I don't think we need a separate deprecated-pluggable.php file though, just keep the if ( ! function_exists() ) wrapper and put the functions in deprecated.php

comment:2 @dd326 years ago

  • Version set to 2.9

I don't think we need a separate deprecated-pluggable.php file though, just keep the if ( ! function_exists() ) wrapper and put the functions in deprecated.php

That'll break anything overriding them.. Which is pointless overriding them anyway as they wont work..

Deprecated include: http://core.trac.wordpress.org/browser/trunk/wp-settings.php#L356

Pluggable include: http://core.trac.wordpress.org/browser/trunk/wp-settings.php#L576

200 lines means a lot in wp-settings :)

comment:3 @nacin6 years ago

dd32: Exactly.

Will do up a patch sometime today accounting for all of this, and will solicit any additional feedback off of that. Only issue is conflicts with other patches in #7493 and possibly #11386.

comment:4 @dd326 years ago

I should've added a IMO in there too.. Leave the deprecated functions in pluggable.php, preferably at the end of the file (as it's the last thing people will see, and hopefully come accross the new version higher up in the file), Just add a deprecated call to it.. and marked clearly that it shouldnt be used at all if at all possible.

@nacin6 years ago

Formally deprecates 10 functions: get_attachment_innerHTML, get_attachment_icon, get_attachment_icon_src, get_the_attachment_link, ngettext_noop, ngettext, get_link, wp_dropdown_cats, dropdown_link_categories, dropdown_categories

comment:5 @nacin6 years ago

Okay, this large patch performs the following:

Formally deprecates 10 functions in four files: wp-includes/l10n.php, post-template.php, bookmark.php, and admin/includes/template.php. Also, wp_login() in pluggable.php now has the _deprecated_function() call (and left in pluggable).

Additionally, the patch cleans up some phpdoc omitted from the commits (or possibly the patches) in #10920 and #7493.

comment:6 @nacin6 years ago

  • Keywords has-patch added; needs-patch dev-feedback removed

Realized there wasn't anything that required feedback to formally deprecate wp_specialchars (for esc_html), attribute_escape (for esc_attr), js_escape (for esc_js), or sanitize_url (for esc_url_raw), so I've created a second patch that includes these as well.

@nacin6 years ago

Second patch includes four pre-2.8 escape functions. These were already marked as deprecated in phpdoc.

@nacin6 years ago

Third patch (these are culmulative) removes dropdown_link_categories from wp-admin/includes/template.php. It had been added to deprecated.php but not removed from its original position. All other functions double-checked.

comment:7 @nacin5 years ago

New patch now moves a total of 15 functions to deprecated.php.

Refreshed patch (r12593) includes translate_with_context(). This had _c() as an alias. I've marked it as deprecated in favor of _x(), though a committing dev may wish to mark it as deprecated in favor of translate_with_gettext_context() instead.

This does mean that using _c() would generate two deprecated function warnings, but that should be fine. The other option is to modifying _c() to not rely on translate_with_context().

Speaking of -- is there any reason why we have yet to deprecate translate_with_gettext_context() and translate() and move all of their references to _x() and __()?

@nacin5 years ago

Deprecates translate_with_context() in favor of _x(). Commiting dev may wish to change reference to _x() to translate_with_gettext_context().

comment:8 @sirzooro5 years ago

  • Cc sirzooro added

Related: #11798

I would not move deprecated functions from wp-admin/includes/template.php to wp-includes/deprecated.php - instead I would like to move them to new file wp-admin/includes/deprecated.php, which will be included from wp-admin/includes/admin.php. These functions are for admin section only, so do not move them to common part.

There is one more function to deprecate: login_pass_ok() in xmlrpc.php. Unfortunately it is a method of wp_xmlrpc_server class, so we cannot move it to deprecated.php. Please mark it with _deprecated_function() call only.

comment:9 @nacin5 years ago

Some functions in wp-includes/deprecated.php came from wp-admin, though I do understand what you're saying. If we are to account for that, I would suggest an is_admin() check around the function define, instead of a new file.

I would have recommended the same thing for the new multisite ms-deprecated.php file, but it'll be much easier to maintain a separate file during the merge, and also easier for MU site admins and plugin authors to transition to WP+MS if they're all in the same place.

I've noticed login_pass_ok() before but I'm not sure how the print of a PHP notice would affect an XMLRPC request so I left it alone.

comment:10 follow-up: @nacin5 years ago

Found a few others. The pre-2.8 widget API, register_sidebar_widget() and register_widget_control(), should be marked as deprecated in favor of wp_register_sidebar_widget() and wp_register_widget_control().

unregister_sidebar_widget() and unregister_widget_control() are straight aliases for their wp_* counterparts. Probably can be deprecated as well.

These functions are already kind of marked with a PHP comment but they should get _deprecated_function() calls added to them and get the deprecated.php treatment.

comment:11 in reply to: ↑ 10 @scribu5 years ago

Replying to nacin:

Found a few others. The pre-2.8 widget API, register_sidebar_widget() and register_widget_control(), should be marked as deprecated in favor of wp_register_sidebar_widget() and wp_register_widget_control().

unregister_sidebar_widget() and unregister_widget_control() are straight aliases for their wp_* counterparts. Probably can be deprecated as well.

These functions are already kind of marked with a PHP comment but they should get _deprecated_function() calls added to them and get the deprecated.php treatment.

+1

comment:12 @scribu5 years ago

  • Keywords commit added

comment:13 @nacin5 years ago

  • Priority changed from low to normal

keywords commit added

Current patch is guaranteed to be stale (if only because we now have wp-admin/deprecated.php and in another ticket we moved a few functions there), and I've mentioned numerous functions that I haven't bothered to patch for yet.

If a core dev can hop in here the same day I patch it up, we can do a big move into wp-includes/deprecated.php and wp-admin/deprecated.php all at once.

comment:14 @nacin5 years ago

(In [13093]) Move deprecated functions to deprecated.php. Deprecate get_the_attachment_link() for wp_get_attachment_link(), get_attachment_icon_src() for wp_get_attachment_image_src(),
get_attachment_icon() and get_attachment_innerHTML() for wp_get_attachment_image(), get_link() for get_bookmark(). Add missing deprecated version numbers. Add inline documentation to pluggable functions that are deprecated. See #11388

comment:15 @nacin5 years ago

(In [13096]) Deprecate old l10n and sanitization APIs. Deprecate ngettext() for _n(), ngettext_noop() for _n_noop(), translate_with_context() for _x(). Deprecate sanitize_url for esc_url_raw, js_escape for esc_js, wp_specialchars for esc_html, attribute_escape for esc_attr. See #11388

comment:16 @nacin5 years ago

(In [13098]) Move deprecated pre-2.8 widget API to deprecated.php. Deprecate register_sidebar_widget, unregister_sidebar_widget, register_widget_control, unregister_widget_control, in favor of their wp_* counterparts. See #11388

comment:17 @nacin5 years ago

(In [13106]) Don't use deprecated functions. see #11388

comment:18 @nacin5 years ago

(In [13097]) Deprecate old category admin template functions. Deprecate dropdown_categories(), dropdown_link_categories(), wp_dropdown_cats() in favor of wp_category_checklist, wp_link_category_checklist, wp_dropdown_categories. See #11388

Looks like that one never posted here.

comment:19 @nacin5 years ago

(In [13204]) Move deprecated pluggable functions to a new file to lower their profile. Also throw deprecated warnings if a plugin defines a deprecated pluggable function. See #11388

comment:20 @nacin5 years ago

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

Six changesets later, I have nothing left for this ticket at this point. Other functions to be deprecated can be addressed in new tickets.

Note: See TracTickets for help on using tickets.