Make fields sortable #142
Make fields sortable #142
Conversation
inc/class-formatter.php
Outdated
| @@ -42,21 +42,39 @@ public function __construct( &$assoc_args, $fields = null, $prefix = false ) { | |||
| * | |||
| * @param array $items | |||
| */ | |||
| public function display_items( $items, $include_total = true ) { | |||
| public function display_items( $order, $orderby, $items, $include_total = true ) { | |||
There was a problem hiding this comment.
Changing this argument order is a backwards compatibility break (however unlikely it may be). Can we add the arguments to the end?
inc/class-command.php
Outdated
| @@ -70,6 +70,13 @@ class Command { | |||
| * | |||
| * [--format=<format>] | |||
| * : Render output in a particular format. | |||
| * | |||
| * [--order=<order>] | |||
| * : Ascending or Descending order. ASC|DESC. | |||
There was a problem hiding this comment.
Can we use the YAML doc annotation to indicate available options?
inc/class-command.php
Outdated
| * : Ascending or Descending order. ASC|DESC. | ||
| * | ||
| * [--orderby=<orderby>] | ||
| * : Order by fields. |
There was a problem hiding this comment.
What's the default order?
There was a problem hiding this comment.
The default order is ASC and there is no default orderby
| $focus = Utils\get_flag_value( $assoc_args, 'all', isset( $args[0] ) ? $args[0] : null ); | ||
|
|
||
| $order = Utils\get_flag_value( $assoc_args, 'order', 'ASC' ); | ||
| $orderby = Utils\get_flag_value( $assoc_args, 'orderby', null ); |
There was a problem hiding this comment.
Does this change the default display?
There was a problem hiding this comment.
No it does not @danielbachhuber
inc/class-formatter.php
Outdated
|
|
||
| if ( $orderby ) { | ||
| usort( $items, function( $a, $b ) use ( $order, $orderby ) { | ||
| if ( 'ASC' === $order ) { |
There was a problem hiding this comment.
Instead of having two separate code paths for the two different sort orders, you could also swap them as needed, and then have 1 unified code path after that.
Something like this:
usort( $items, function( $a, $b ) use ( $order, $orderby ) {
list( $first, $second ) = 'ASC' === $order ? array( $a, $b ) : array( $b, $a );
if ( is_numeric( $first->$orderby ) && is_numeric( $second->$orderby ) ) {
return $this->compare_float( $first->$orderby, $second->$orderby );
}
return strcmp( $first->$orderby, $second->$orderby );
});
|
@Sidsector9 Planning to wrap this pull request up, or would you like help getting it over the finish line? |
|
@danielbachhuber Give me a day more, I'll finish this |
schlessera
dismissed
danielbachhuber’s stale review
Change requested was addressed.
schlessera
changed the title
GH#36 Add sort feature by field type and orderby
See #36