Introduce helper function `wp_list_sort()`

[flixos90]

When dealing with object lists in WordPress, we have easy-to-use utility functions available for filtering (wp_list_filter()) and plucking a field (wp_list_pluck()). What we don't have however, is a function to easily sort the list by a specific field.

Although a function like this wouldn't need to be used inside of WordPress Core, it would certainly make it easier for plugin developers by being provided such a function. Therefore I think we should add it.

[flixos90]

37128.diff​ adds a function wp_list_sort().

The patch also adds another private function _wp_list_sort_callback() which is used by usort() in the main function. The fields to order by are temporarily stored in a private global $_wp_list_sort which is only available while wp_list_sort() is doing its work.

As far as the parameters work, they are similar like the $orderby and $order arguments of WP_Query for example. One can either pass $orderby as a string (field name) and $order (ASC or DESC) or alternatively only specify $orderby as an array of orderby => order to support sorting by multiple fields.

[swissspidy]

Using globals is bad, using temporary globals seems even worse. With initiatives like #37699, we should avoid introducing new globals.

How would this look with a helper class for plucking, filtering and sorting? Or maybe create_function() (sigh)?

Introducing wp_list_sort() without using it in core is not ideal, but an extensible helper class for interacting with arrays and objects might be more convincing.

[flixos90]

@swissspidy: Thanks for the feedback, good point. In 37128.2.diff​ I introduce a new class WP_List_Util that takes care of that functionality. It also takes care of filtering and plucking, so the original functions only act as wrappers now. This way we have a clear structure for also performing multiple operations on one list.

[DrewAPicture]

WP_List_Util is certainly interesting. I think I'd like to see us bail early in the filter() method if the $operator doesn't match a whitelist. Also still needs unit tests :-)

[swissspidy]

37128.3.diff​ adds unit tests and bails early in filter() if the $operator is not and, or or not.

[jorbin]

In general, core has tended to not add utility functions that it doesn't use. Is there anywhere in core this would make sense right now? Not saying that should keep it out, but I think if core can use it, the case for including it is stronger.

One thing that isn't obvious from me just reading the tests and the docs is what happens if wp_list_sort is called on an object where $orderby isn't set on each field? Where does that fall in the sort? Seems like that is something that should be documented and tested so the code remains consistent into the future.

Additionally, what about when you want to do both asc and desc ? For example, alphabetical by title, but reverse chronological? Perhaps there should just be a single argument that you pass in that includes both the field and the direction?

[flixos90]

Replying to jorbin:

In general, core has tended to not add utility functions that it doesn't use. Is there anywhere in core this would make sense right now? Not saying that should keep it out, but I think if core can use it, the case for including it is stronger.

Good point, I was actually planning to look for use-cases earlier. I just checked and found several places where using wp_list_sort() would make sense. We could get rid of the following functions/methods (or at least not use them anymore):

• _usort_terms_by_name()
• _usort_terms_by_ID()
• _sort_nav_menu_items()
• WP_Customize_Manager::_cmp_priority()
• WP_Customize_Widgets::_sort_name_callback()

There might be more, but that's what I found now. So there would indeed be a use for the function in Core. I can update another patch later today or tomorrow to illustrate what the implementation could look like.

One thing that isn't obvious from me just reading the tests and the docs is what happens if wp_list_sort is called on an object where $orderby isn't set on each field? Where does that fall in the sort? Seems like that is something that should be documented and tested so the code remains consistent into the future.

If the $orderby field isn't set on one of the objects/arrays compared, the comparison is skipped, basically resulting in an equal comparison. In this case the next field compared (if using an array of $orderby => $order) will decide. If all comparisons between two objects result in an equal check, the whole check will cause an equal result (i.e. return 0). Since PHP only does unstable sort, this will unfortunately randomize the result. However we can improve this if needed, for example by adding an internal __index property on each item in the array (and removing it again after the comparison) and then fallback to it to maintain the original order on an equal comparison.

Additionally, what about when you want to do both asc and desc ? For example, alphabetical by title, but reverse chronological? Perhaps there should just be a single argument that you pass in that includes both the field and the direction?

In the patch you can do that already: you can specify the $orderby parameter as an array of (multiple) $orderby => $order pairs (in this case it will skip the third $order parameter). Is that what you meant or did I misunderstand you?

[swissspidy]

Replying to flixos90:

One thing that isn't obvious from me just reading the tests and the docs is what happens if wp_list_sort is called on an object where $orderby isn't set on each field? Where does that fall in the sort? Seems like that is something that should be documented and tested so the code remains consistent into the future.

If the $orderby field isn't set on one of the objects/arrays compared, the comparison is skipped, basically resulting in an equal comparison. In this case the next field compared (if using an array of $orderby => $order) will decide. If all comparisons between two objects result in an equal check, the whole check will cause an equal result (i.e. return 0). Since PHP only does unstable sort, this will unfortunately randomize the result. However we can improve this if needed, for example by adding an internal __index property on each item in the array (and removing it again after the comparison) and then fallback to it to maintain the original order on an equal comparison.

I don't really think we should go as far as adding internal properties and such and simply let PHP handle this.

Additionally, what about when you want to do both asc and desc ? For example, alphabetical by title, but reverse chronological? Perhaps there should just be a single argument that you pass in that includes both the field and the direction?

In the patch you can do that already: you can specify the $orderby parameter as an array of (multiple) $orderby => $order pairs (in this case it will skip the third $order parameter). Is that what you meant or did I misunderstand you?

What about removing that third parameter and always requiring a key => value pair?

[flixos90]

Replying to swissspidy:

What about removing that third parameter and always requiring a key => value pair?

I was aiming for parity with how all the *_Query classes handle their order arguments, but I'm certainly open to remove that parameter.

[flixos90]

Usage examples for wp_list_sort() in Core

[flixos90]

37128-usage.2.diff​ includes some use-cases implemented for wp_list_sort() in Core. When testing make sure to also merge the patch for the actual functionality (since this patch only includes implementation).

[swissspidy]

37128-usage.2.diff​ should also remove/deprecate the now unneeded sorting functions in these files (dead code).

[flixos90]

Replying to swissspidy:

37128-usage.2.diff​ should also remove/deprecate the now unneeded sorting functions in these files (dead code).

Deprecate or remove? I would assume we can remove the ones that are private/protected (although they could theoretically be used by someone extending a class).

[swissspidy]

Replying to flixos90:

Replying to swissspidy:

37128-usage.2.diff​ should also remove/deprecate the now unneeded sorting functions in these files (dead code).

Deprecate or remove? I would assume we can remove the ones that are private/protected (although they could theoretically be used by someone extending a class).

Private methods could be removed straight away (not inherited), but at first glance it looks like these are only public methods. So maybe deprecating them first before removing them early in the next release.

[flixos90]

In 37128-usage.3.diff​ all previously used methods and functions have been deprecated. All of these are either functions marked private or protected methods, so we can probably remove them quickly, like one release after the deprecated notice. Still, for now all of them remain and are only deprecated.

What would be the next steps for this ticket? Anyone who would be willing to review this one? :)

[swissspidy]

If @DrewAPicture can approve the docs / changes in 37128-usage.3.diff​, I'm happy to commit this :-)

[jorbin]

@swissspidy Let's land it.

[swissspidy]

In 38859:

General: Introduce a wp_list_sort() helper function.

In addition to wp_list_filter() for filtering a list of objects, and wp_list_pluck() for plucking a certain field out of each object in a list, this new function can be used for sorting a list of objects by specific fields. These functions are now all contained within the new WP_List_Util() class and wp_list_sort() is used in various parts of core for sorting lists.

Props flixos90, DrewAPicture, jorbin.
Fixes #37128.

[ocean90]

@flixos90, @swissspidy There are a few failing tests, see ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/169534080.

[westonruter]

In 38862:

Customize: Revert part of [38859] which caused sections to get deactivated in the customizer.

See #37128.

[westonruter]

See active-sections-before-r38859.png​ and active-sections-after-r38859.png​ for what the partial revert in [38862] fixed. There was an issue in [38859] where the new sorting function was corrupting the list of sections.

[swissspidy]

@westonruter Thanks for the screenshots. wp_list_sort() right now only does a usort(), whereas the affected customizer parts sometimes use uasort(). [38862] reverts an instance of usort() as well though. With that being said, I'd rather have reverted the whole commit because of the test failures instead.

The tests are failing because usort() / uasort() are not stable and their behaviour changed from PHP 5 to PHP 7. That makes it basically untestable.

What we definitely need is wp_list_usort() and wp_list_uasort() or an additional parameter to wp_list_sort().

[jorbin]

@swissspidy Can you finish the revert and we can continue work on this as a patch?

[ocean90]

In 38863:

Revert [38859] due to an incomplete implementation.

See https://core.trac.wordpress.org/ticket/37128#comment:27.
See #37128.

[flixos90]

37128.4.diff​ fixes the failing unit tests (related to PHP unstable sorting). It also adds a $preserve_keys parameter to wp_list_sort() (if true, uasort() is used). This parameter is provided as such in the relevant functions in WP_Customize_Manager.

Note that this patch now contains both the basic implementation plus the usage in the rest of Core to make maintenance easier.

[ocean90]

Travis CI build for 37128.4.diff​: ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/169711013 (passed)

Can we have a test which covers [38862]?

[flixos90]

@ocean90 Will do that later this weekend.

[flixos90]

37128.5.diff​ adds a unit test test_wp_list_sort_preserve_keys() to test the new $preserve_keys functionality (using uasort()).

[swissspidy]

Travis CI of the latest patch looks good: ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/170390880.

@westonruter Wanna test this with the customizer again?

[ocean90]

Replying to swissspidy:

@westonruter Wanna test this with the customizer again?

Said this already to @flixos90: The test_wp_list_sort_preserve_keys() is good but my intention was to have a test which covers the sorting behaviour for the customizer specifically.

[flixos90]

In 37128.6.diff​ I added three unit tests for the customizer that checks that the sorted objects still have valid keys for controls, sections and panels respectively.

[ocean90]

In 38928:

General: Introduce a wp_list_sort() helper function, v2.

In addition to wp_list_filter() for filtering a list of objects, and wp_list_pluck() for plucking a certain field out of each object in a list, this new function can be used for sorting a list of objects by specific fields. These functions are now all contained within the new WP_List_Util() class and wp_list_sort() is used in various parts of core for sorting lists.

This was previously committed in [38859] but got reverted in [38862] and [38863]. To fix the previous issues, wp_list_sort() supports now an additional argument to preserve array keys via uasort().

Props flixos90, DrewAPicture, jorbin.
Fixes #37128.

[desrosj]

Linking the dev note that was published here for reference: ​https://make.wordpress.org/core/2016/11/02/wp_list_sort-and-wp_list_util-in-4-7/