WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 23 months ago

#16768 closed enhancement (fixed)

PHP5-port - Class Constructors Review for 3.2

Reported by: hakre Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

Related: #10861, #14920 and others.

@ryan: my name counts as anybody else' one.

Attachments (12)

16768.patch (493 bytes) - added by hakre 3 years ago.
originating by hakre @ #10861
16768.2.patch (1.1 KB) - added by hakre 3 years ago.
some in wp-includes/capabilities.php
16768.3.patch (998 bytes) - added by hakre 3 years ago.
wpdb PHP4 constructor removal, Eelated: #16764; X-Eef: #14672, [15537]
16768.4.patch (489 bytes) - added by hakre 3 years ago.
cache.php constructor; originating by hakre @ #10861
media_php4_constructor.patch (509 bytes) - added by Jayjdk 3 years ago.
Removes PHP4 constructor for WP_Embed in media.php
16768.class-oembed.patch (479 bytes) - added by ocean90 3 years ago.
widgets.16768.diff (6.3 KB) - added by scribu 3 years ago.
16768.alot.patch (23.4 KB) - added by ocean90 3 years ago.
text_diff.16768.patch (1.4 KB) - added by ampt 3 years ago.
text_diff.16768.2.patch (3.0 KB) - added by ampt 3 years ago.
text_diff.16768.3.patch (398 bytes) - added by ampt 3 years ago.
mixed-constructor-test.php (203 bytes) - added by markjaquith 3 years ago.
Neat.

Download all attachments as: .zip

Change History (34)

hakre3 years ago

originating by hakre @ #10861

hakre3 years ago

some in wp-includes/capabilities.php

comment:1 hakre3 years ago

  • Keywords dev-feedback added

@todo WP_User tries to return a non-object. Throw an exception?

comment:2 hakre3 years ago

  • Summary changed from Class Constructors Review for 3.2 to PHP5-port - Class Constructors Review for 3.2

hakre3 years ago

wpdb PHP4 constructor removal, Eelated: #16764; X-Eef: #14672, [15537]

hakre3 years ago

cache.php constructor; originating by hakre @ #10861

comment:3 hakre3 years ago

Related: #16782 - contains constructor changes for WP_Error, interesting case for having the constructor "returning" something (e.g. empty string) which is not possible and can be seen in other constructors inside wordpress as well.

comment:4 nacin3 years ago

  • Type changed from defect (bug) to enhancement

comment:5 ryan3 years ago

  • Milestone changed from Awaiting Review to 3.2

comment:6 ryan3 years ago

(In [17604]) Remove PHP4 constructors. Props hakre. see #16768

Jayjdk3 years ago

Removes PHP4 constructor for WP_Embed in media.php

comment:7 ryan3 years ago

(In [17631]) Remove PHP4 constructor. Props Jayjdk. see #16768

ocean903 years ago

comment:8 ocean903 years ago

16768.class-oembed.patch should be the last one.

scribu3 years ago

comment:9 scribu3 years ago

widgets.16768.diff moves widget classes to __construct().

The PHP4 constructor in WP_Widget can't be removed because it's likely used by plugins.

Patch also fixes the way WP_Widget::WP_Widget() calls __construct(), avoiding an endless loop.

comment:10 ryan3 years ago

(In [17654]) Remove PHP4 constructor. Props ocean90. see #16768

comment:11 ryan3 years ago

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

(In [17655]) Remove PHP4 constructors. Props scribu. fixes #16768

ocean903 years ago

comment:12 ocean903 years ago

  • Keywords dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

16768.alot.patch: Some more. Needs a second review, I couldn't find any problems with that.

comment:13 ryan3 years ago

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

(In [17771]) Constructor cleanup. Props ocean90. fixes #16768

ampt3 years ago

ampt3 years ago

comment:14 ampt3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

text_diff.16768.2.patch: Found a few more in Text Diff classes

comment:15 ocean903 years ago

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

Thanks for your patch, but I asked ryan yesterday and he tolds me, to leave third party classes as it is.

comment:16 ampt3 years ago

Ok no problem, thanks.

ampt3 years ago

comment:17 ampt3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Sorry, one more thing. Does this count?
text_diff.16768.3.patch

Fix the call to the parent's constructor in WP_Text_Diff_Renderer_Table

I am getting error's with a subclass WP_Text_Diff_Renderer_Table.

comment:18 ocean903 years ago

Interesting, yes that works. I leave that, because I thought, that it won't work because of the missing __construct() in the parent class.

No errors before and after your patch. Which errors do you mean?

comment:19 ampt3 years ago

The errors are from a plugin which uses a subclass of WP_Text_Diff_Renderer so its not a part of core, but this affects anyone wanting to subclass WP_Text_Diff_Renderer.

As far as PHP5 is concerned a constructor is a constructor, it doesn't seem to matter if the constructor is defined as the class name or defined as __construct.

Version 0, edited 3 years ago by ampt (next)

markjaquith3 years ago

Neat.

comment:20 markjaquith3 years ago

In [17781]:

More constructor cleanup. Props ampt. see #16768

comment:21 jane3 years ago

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

Freeze!

Note: See TracTickets for help on using tickets.