Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#33854 closed defect (bug) (fixed)

Make WP_List_Table->get_primary_column_name() public

Reported by: greglone's profile GregLone Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.3
Component: Administration Keywords: needs-docs
Focuses: ui Cc:

Description

There are currently no way to know which column is primary "from the outside".
For example, I want to add a custom column in the posts list and place it right after the primary column: I can't.
Moreover, I can't see the point of declaring this method protected.

Change History (10)

#1 @GregLone
9 years ago

  • Keywords needs-patch dev-feedback added

Weird. I was a bout to submit a patch but it seems that declaring this method public makes the page not to load. oO

#2 @wonderboymusic
9 years ago

  • Milestone changed from Awaiting Review to 4.4
  • Owner set to wonderboymusic
  • Status changed from new to assigned

#3 @wonderboymusic
9 years ago

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

In 34101:

Make WP_List_Table::get_primary_column_name() public in list table classes that have it.

Fixes #33854.

#4 @GregLone
9 years ago

That was fast :o
Thank you @wonderboymusic!

This ticket was mentioned in Slack in #core by nerrad. View the logs.


9 years ago

#6 @nerrad
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

While I totally dig the value of this change and can understand if it stays as is, I think it'd be useful to try and keep access levels for methods consistent between WP versions. The issue is if plugins extend WP_List_Table (and yes I know internally its considered "private" but good luck trying to enforce that :) ), and have overridden any methods, then when you change the access level that means that fatal errors will start getting thrown like this:

PHP Fatal error:  Access level to EE_Admin_List_Table::get_primary_column_name() must be public (as in class WP_List_Table)

If access levels keep changing, then plugins cannot reliably override methods from WP_List_Table.

One thing that might be beneficial is to have a standard for protected and private method names (i.e. _get_primary_column_name) and then if a request like this comes in one can just create a public getter for it. Too late to do here but I still like the idea of doing a public wrapper to retrieve the value from the original protected method.

So maybe something like get_the_primary_column_name?

Last edited 9 years ago by nerrad (previous) (diff)

#7 @wonderboymusic
9 years ago

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

In 34128:

In WP_List_Table, make a new public method, ->get_primary_column(), and revert [34101] due to BC issues.

Fixes #33854.

#8 @TobiasBg
9 years ago

  • Keywords needs-docs added; needs-patch dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

get_primary_column() needs some docs as well, including maybe a note about it being a public wrapper function.

#9 @wonderboymusic
9 years ago

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

In 34166:

Add a doc block to WP_List_Table::get_primary_column().

Fixes #33854.

#10 @DrewAPicture
9 years ago

In 34229:

Docs: Remove markdown from the DocBlock summary for WP_List_Table::get_primary_column(), introduced in [34166].

The PHP inline documentation standards for summaries call for not using an markup or markdown. Also, using the full Class::method() in this context allows for better clarity in what is being referenced.

See #33854. See #32246.

Note: See TracTickets for help on using tickets.