[wp-trac] [WordPress Trac] #45029: Changes by widget_{$this->id_base}_instance_schema are overridden by sub class.

WordPress Trac noreply at wordpress.org
Tue Apr 2 17:34:40 UTC 2019


#45029: Changes by widget_{$this->id_base}_instance_schema  are overridden by sub
class.
-------------------------------------------------+-------------------------
 Reporter:  Toro_Unit                            |       Owner:
                                                 |  SergeyBiryukov
     Type:  defect (bug)                         |      Status:  reviewing
 Priority:  normal                               |   Milestone:  5.2
Component:  Widgets                              |     Version:  4.9
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests dev-        |     Focuses:
  feedback                                       |
-------------------------------------------------+-------------------------

Comment (by birgire):

 Replying to [comment:10 SergeyBiryukov]:
 > Replying to [comment:9 birgire]:
 > > We can e.g. compare it with {{{wp_parse_args( $options, $defaults
 )}}}, that's a wrapper of {{{array_merge( $defaults, $options )}}},
 regarding the expected input order.
 >
 > Could we use `wp_parse_args()` here as well, for consistency with the
 rest of core? That should prevent further confusion.

 yes I think that would be possible, though we would also add support for
 string and object type of arguments.

 As the use of {{{wp_parse_args}}} raises some other questions (see below),
 I would suggest we use the patch [attachment:"45029-2.diff"] as is, until
 it's more clear what to do with the type hinting of {{{@param}}} :-)

 ----

 I had a look at some {{{wp_parse_args}}} core uses and it seems there's no
 specific rule in place regarding how to document the input type; sometimes
 the type is documented as an {{{array}}}, but sometimes as
 {{{array|string}}}. It seems the {{{object}}} support is never documented,
 apart from the {{{array|string|object}}} input type of the
 {{{wp_parse_args}}} function.

 This raises the question, how faithful the type-hinting for {{{@param}}}
 is in general to the actual supported types.

 If I recall correctly, the general aim is to support array input
 arguments, rather than string parse the input.

 If we change it to {{{wp_parse_args}}}, wouldn't we also modify inline
 documentation for the filter unchanged:

 https://core.trac.wordpress.org/browser/tags/5.1.1/src/wp-includes/widgets
 /class-wp-widget-media.php#L156

 But it would be nice to have a guideline on this matter in the Handbook:

 https://make.wordpress.org/core/handbook/best-practices/inline-
 documentation-standards/php/

 ----

 ps: The inline documentation of {{{wp_parse_args}}} says:

 https://core.trac.wordpress.org/browser/tags/5.1.1/src/wp-
 includes/functions.php#L3943

 {{{This function is used throughout WordPress to allow for both string or
 array to be merged into another array.}}}

 but I think we should consider updating it as well (another ticket) to
 mention the object support.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/45029#comment:11>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list