WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#5418 closed defect (bug) (fixed)

remove unused variables from wp-admin/*.php

Reported by: DD32 Owned by:
Milestone: 2.5 Priority: normal
Severity: normal Version: 2.5
Component: General Keywords: has-patch
Focuses: Cc:

Description

In short, This patch removes unused variables from various admin files.

/*.php, /wp-includes/*.php to follow in coming days, I'm currently working through a automated audit of code for unused variables/PHP versions required for functions/constants

Attachments (7)

5418.unused-variables.diff (6.3 KB) - added by DD32 8 years ago.
5418.wp-admin.2.diff (7.1 KB) - added by DD32 8 years ago.
typos, Missed items
5418.wp-includes.root.diff (61.2 KB) - added by DD32 8 years ago.
/wp-includes/*.php + /*.php variable cleanup + indenting + typos
5418.s-apply_filters-do_action.diff (1.4 KB) - added by DD32 8 years ago.
5418.xmlrpc.diff (5.4 KB) - added by DD32 8 years ago.
no-commit: Shows edits to xmlrpc.php without spacing modifications
5418.rep4.diff (14.4 KB) - added by DD32 8 years ago.
4th iteration over files.
5418.rep4.2.diff (13.0 KB) - added by DD32 8 years ago.

Download all attachments as: .zip

Change History (28)

@DD328 years ago

comment:1 @DD328 years ago

  • Keywords has-patch added

comment:2 @ryan8 years ago

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

(In [6354]) Remove unused variables. Props DD32. fixes #5418

comment:3 follow-up: @Bobcat8 years ago

Looks like a double declaration in upload.php, line 81:
global $style, $style;

comment:4 in reply to: ↑ 3 @DD328 years ago

Replying to Bobcat:

Looks like a double declaration in upload.php, line 81:
global $style, $style;

Er, Thanks for catching that one :)

I'll include it in the next patch if its not removed by then.

comment:5 @Nazgul8 years ago

There is also a small typo in the patch:
$depreciated instead of $deprecated.

@DD328 years ago

typos, Missed items

comment:6 @DD328 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Fixed up a few more & corrected the mis-spellings.

just a note on this 2nd patch:

  • The nonce field is removed as it never has a value, seems the field is added furthur down the page anyway
  • Theres some testing stuff at the end of {{{wp-admin/install-helper.php))) left over from removing a link-table column, i've left that in place.
  • I'm marking function arguements as $deprecated if they're no longer used within the function itself. If it seems like the arguement should be used but doesnt, i'll open a ticket for the item.

comment:7 @ryan8 years ago

(In [6363]) Remove unused variables. Props DD32. see #5418

comment:8 @DD328 years ago

Attaching another patch here, even though ticket specifies wp-admin.

Attached ticket covers wp-includes and root files.

xmlrpc.php looks ugly in the patch, the mixture of Tab & 2space indents were changed into tabs. Infact thats the same in a few places where some indentings were corrected.

A few typos were corrected as well (see 2nd wp-app.php & 3rd wp-includes\formatting.php edits)

This is a huge patch, So It'll probably be good if others can do a glance review of any files they've got good knowledge of, While i'm pretty sure i've not broken anything, as the last patch showed, my spelling isnt that great sometimes :)

3rd party classes and admin import files were ignored once again.

@DD328 years ago

/wp-includes/*.php + /*.php variable cleanup + indenting + typos

comment:9 @ryan8 years ago

(In [6364]) Remove unused vars. Props DD32. see #5418

comment:10 follow-up: @Nazgul8 years ago

That last patch introduced this:
apply_filters('wp_create_file_in_uploads', $file); replicate

The output of the filter was removed, because it wasn't used in the rest of the code, but doesn't that imply that the entire apply_filters call can be removed as well? Why filter if you throw away the output.

This happens in wp-app.php (415), wp-admin/custom-header.php (231) and wp-admin/link-import.php (126).

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

Replying to Nazgul:

That last patch introduced this:
apply_filters('wp_create_file_in_uploads', $file); replicate

Cheers, I forgot to ask about that..

I didnt see the point in storing the return value, but I thought it might currently be in use by plugins/themes.
Eg. a quick google: http://www.google.com/search?hl=en&safe=off&client=opera&rls=en&hs=Qxt&q=add_filter%28+%27wp_create_file_in_uploads%27&btnG=Search seems to suggest K2 uses it to process the custom header image.

comment:12 follow-up: @JeremyVisser8 years ago

In that case, do_action('wp_create_file_in_uploads', $file); would suffice.

comment:13 in reply to: ↑ 12 ; follow-up: @DD328 years ago

Replying to JeremyVisser:

In that case, do_action('wp_create_file_in_uploads', $file); would suffice.

I thought so too, However, That filter is used elsewhere, and in those cases, the output may be modified.

In the lines pointed out, the return value was simply going to be ignored anyway, Yet i thought other scripts may want to hook those events too.

comment:14 in reply to: ↑ 13 ; follow-up: @JeremyVisser8 years ago

Replying to DD32:

However, that filter is used elsewhere, and in those cases, the output may be modified.

But in this instance, the value is being ignored, so semantically, I believe it's better to do do_action(). (Although, I think do_action() uses apply_filters() internally, so it's definitely not for optimisation reasons.)

comment:15 in reply to: ↑ 14 @DD328 years ago

Replying to JeremyVisser:

But in this instance, the value is being ignored, so semantically, I believe it's better to do do_action(). (Although, I think do_action() uses apply_filters() internally, so it's definitely not for optimisation reasons.)

Ah your right, I for some reason had it in my mind, that the action, and filters were stored in seperate arrays.

I'll make the change and then post a patch back.
I'll also add a diff of xmlrpc.php so as to show what was removed from it when spacings are ignored. (not intended to be commited -- just for information purposes)

@DD328 years ago

no-commit: Shows edits to xmlrpc.php without spacing modifications

@DD328 years ago

4th iteration over files.

comment:16 @DD328 years ago

4th iteration over files. Fixes some typo's around the place, adds more of the deprecated marks.

comment:17 follow-up: @ryan8 years ago

Actions and filter hooks such as update_option_home, update_option_siteurl, and cure_update_footer have unused arguments so that they fit the prototype for given action or filter. Instead of saying "deprecated" we should say "unused" or something to that affect.

comment:18 in reply to: ↑ 17 @DD328 years ago

Replying to ryan:

Actions and filter hooks such as update_option_home, update_option_siteurl, and cure_update_footer have unused arguments so that they fit the prototype for given action or filter. Instead of saying "deprecated" we should say "unused" or something to that affect.

Allright, I'll go revert those changes, thanks.

@DD328 years ago

comment:19 @DD328 years ago

Changes applied.

Note the change in the taxonomy too:

- $value = apply_filters("${taxonomy}_$field_rss", $value); 
+ $value = apply_filters("${taxonomy}_${field}_rss", $value); 

At present it attempts to expand the "$field_rss" variable instead of "$field".

comment:20 @ryan8 years ago

(In [6551]) Unused var cleanup. Props DD32. see #5418

comment:21 @ryan7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.