From ad178edb8036f1e87de6b46000894a6047adb33b Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Thu, 20 Mar 2008 17:02:35 +0000 Subject: [PATCH] Move some of the changes I made to Special:Categories out of SpecialCategories.php and into Pager.php. Allow multiple possible sort orders for IndexPager, allow user override of sort direction for IndexPager, add extra links as appropriate for AlphabeticPager. Some of this seems to duplicate TablePager logic; I'm not sure why that's a separate class to begin with. Or why AlphabeticPager is called that, given that it's not necessarily alphabetic at all. In fact all of these child classes seem to perform almost identical functions and it seems as though folding them into the IndexPager class would reduce code duplication. --- RELEASE-NOTES | 6 +- includes/Pager.php | 151 +++++++++++++++++++++++++++--- includes/SpecialCategories.php | 65 ++----------- languages/messages/MessagesEn.php | 4 +- maintenance/language/messages.inc | 4 +- 5 files changed, 155 insertions(+), 75 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 21efa3cf1e..66ca955fc4 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -49,9 +49,11 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * Add category table to allow better tracking of category membership counts ** (bug 1212) Give correct membership counts on the pages of large categories ** Use category table for more efficient display of Special:Categories -** Allow sorting by number of members on Special:Categories, and also allow - descending sorts +** Allow sorting by number of members on Special:Categories * (bug 1459) Search for duplicate files by hash: Special:FileDuplicateSearch +* Allow the user to choose whether to use a descending or ascending sort in + AlphabeticPager-based special pages (Categories, Listusers, Protectedpages/ + titles). === Bug fixes in 1.13 === diff --git a/includes/Pager.php b/includes/Pager.php index 1e5b11cc7c..fafbd9cdbe 100644 --- a/includes/Pager.php +++ b/includes/Pager.php @@ -60,19 +60,34 @@ abstract class IndexPager implements Pager { public $mDb; public $mPastTheEndRow; + /** + * The index to actually be used for ordering. This is a single string e- + * ven if multiple orderings are supported. + */ protected $mIndexField; - + /** For pages that support multiple types of ordering, which one to use. */ + protected $mOrderType; /** - * Default query direction. false for ascending, true for descending + * $mDefaultDirection gives the direction to use when sorting results: + * false for ascending, true for descending. If $mIsBackwards is set, we + * start from the opposite end, but we still sort the page itself according + * to $mDefaultDirection. E.g., if $mDefaultDirection is false but we're + * going backwards, we'll display the last page of results, but the last + * result will be at the bottom, not the top. + * + * "Default" is really a misnomer here, since now it's possible for the + * user to directly modify this with query parameters. TODO: rename that + * to $mDescending, maybe set this to that by reference for compatibility. */ - public $mDefaultDirection = false; + public $mDefaultDirection; + public $mIsBackwards; /** * Result object for the query. Warning: seek before use. */ public $mResult; - function __construct() { + public function __construct() { global $wgRequest, $wgUser; $this->mRequest = $wgRequest; @@ -80,14 +95,44 @@ abstract class IndexPager implements Pager { # arbitrary string to support the widest variety of index types. Be # careful outputting it into HTML! $this->mOffset = $this->mRequest->getText( 'offset' ); - + # Use consistent behavior for the limit options $this->mDefaultLimit = intval( $wgUser->getOption( 'rclimit' ) ); list( $this->mLimit, /* $offset */ ) = $this->mRequest->getLimitOffset(); - + $this->mIsBackwards = ( $this->mRequest->getVal( 'dir' ) == 'prev' ); - $this->mIndexField = $this->getIndexField(); $this->mDb = wfGetDB( DB_SLAVE ); + + $index = $this->getIndexField(); + $order = $this->mRequest->getVal( 'order' ); + if( is_array( $index ) && isset( $index[$order] ) ) { + $this->mOrderType = $order; + $this->mIndexField = $index[$order]; + } elseif( is_array( $index ) ) { + # First element is the default + reset( $index ); + list( $this->mOrderType, $this->mIndexField ) = each( $index ); + } else { + # $index is not an array + $this->mOrderType = null; + $this->mIndexField = $index; + } + + if( !isset( $this->mDefaultDirection ) ) { + $dir = $this->getDefaultDirection(); + $this->mDefaultDirection = is_array( $dir ) + ? $dir[$this->mOrderType] + : $dir; + } + + # FIXME: Can we figure out a less confusing convention than using a + # "direction" parameter when we already have "dir" and "order"? + if( $this->mRequest->getVal( 'direction' ) == 'asc' ) { + $this->mDefaultDirection = false; + } + if( $this->mRequest->getVal( 'direction' ) == 'desc' ) { + $this->mDefaultDirection = true; + } } /** @@ -300,6 +345,8 @@ abstract class IndexPager implements Pager { unset( $this->mDefaultQuery['dir'] ); unset( $this->mDefaultQuery['offset'] ); unset( $this->mDefaultQuery['limit'] ); + unset( $this->mDefaultQuery['direction'] ); + unset( $this->mDefaultQuery['order'] ); } return $this->mDefaultQuery; } @@ -397,10 +444,32 @@ abstract class IndexPager implements Pager { abstract function getQueryInfo(); /** - * This function should be overridden to return the name of the - * index field. + * This function should be overridden to return the name of the index fi- + * eld. If the pager supports multiple orders, it may return an array of + * 'querykey' => 'indexfield' pairs, so that a request with &count=querykey + * will use indexfield to sort. In this case, the first returned key is + * the default. */ abstract function getIndexField(); + + /** + * Return the default sorting direction: false for ascending, true for de- + * scending. You can also have an associative array of ordertype => dir, + * if multiple order types are supported. In this case getIndexField() + * must return an array, and the keys of that must exactly match the keys + * of this. + * + * For backward compatibility, this method's return value will be ignored + * if $this->mDefaultDirection is already set when the constructor is + * called, for instance if it's statically initialized. In that case the + * value of that variable (which must be a boolean) will be used. + * + * Note that despite its name, this does not return the value of the now- + * misnamed $this->mDefaultDirection member variable. That is not a + * default, it's the actual direction used. This is just the default and + * can be overridden by user input. + */ + protected function getDefaultDirection() { return false; } } @@ -409,8 +478,6 @@ abstract class IndexPager implements Pager { * @addtogroup Pager */ abstract class AlphabeticPager extends IndexPager { - public $mDefaultDirection = false; - function __construct() { parent::__construct(); } @@ -422,20 +489,76 @@ abstract class AlphabeticPager extends IndexPager { function getNavigationBar() { global $wgLang; + if( isset( $this->mNavigationBar ) ) { + return $this->mNavigationBar; + } + $linkTexts = array( 'prev' => wfMsgHtml( 'prevn', $wgLang->formatNum( $this->mLimit ) ), 'next' => wfMsgHtml( 'nextn', $wgLang->formatNum($this->mLimit ) ), - 'first' => wfMsgHtml( 'page_first' ), /* Introduced the message */ - 'last' => wfMsgHtml( 'page_last' ) /* Introduced the message */ + 'first' => wfMsgHtml( 'page_first' ), + 'last' => wfMsgHtml( 'page_last' ) ); $pagingLinks = $this->getPagingLinks( $linkTexts ); $limitLinks = $this->getLimitLinks(); $limits = implode( ' | ', $limitLinks ); - $this->mNavigationBar = "({$pagingLinks['first']} | {$pagingLinks['last']}) " . wfMsgHtml("viewprevnext", $pagingLinks['prev'], $pagingLinks['next'], $limits); + $this->mNavigationBar = + "({$pagingLinks['first']} | {$pagingLinks['last']}) " . + wfMsgHtml( 'viewprevnext', $pagingLinks['prev'], + $pagingLinks['next'], $limits ); + + # Which direction should the link go? Opposite of the current. + $dir = $this->mDefaultDirection ? 'asc' : 'desc'; + $query = array( 'direction' => $dir ); + if( $this->mOrderType !== null ) { + $query['order'] = $this->mOrderType; + } + # Note for grep: uses pager-sort-asc, pager-sort-desc + $this->mNavigationBar .= ' (' . $this->makeLink( + wfMsgHTML( "pager-sort-$dir" ), + $query + ) . ')'; + + if( !is_array( $this->getIndexField() ) ) { + # Early return to avoid undue nesting + return $this->mNavigationBar; + } + + $extra = ''; + $first = true; + $msgs = $this->getOrderTypeMessages(); + foreach( array_keys( $msgs ) as $order ) { + if( $order == $this->mOrderType ) { + continue; + } + if( !$first ) { + $extra .= ' | '; + $first = false; + } + $extra .= $this->makeLink( + wfMsgHTML( $msgs[$order] ), + array( 'order' => $order ) + ); + } + + if( $extra !== '' ) { + $this->mNavigationBar .= " ($extra)"; + } + return $this->mNavigationBar; + } + /** + * If this supports multiple order type messages, give the message key for + * enabling each one in getNavigationBar. The return type is an associa- + * tive array whose keys must exactly match the keys of the array returned + * by getIndexField(), and whose values are message keys. + * @return array + */ + protected function getOrderTypeMessages() { + return null; } } diff --git a/includes/SpecialCategories.php b/includes/SpecialCategories.php index 6ffe65b453..38996dcd4a 100644 --- a/includes/SpecialCategories.php +++ b/includes/SpecialCategories.php @@ -21,21 +21,6 @@ function wfSpecialCategories() { * @addtogroup Pager */ class CategoryPager extends AlphabeticPager { - private $mOrderType = 'abc'; - - public function __construct() { - parent::__construct(); - if( $this->mRequest->getText( 'order' ) == 'count' ) { - $this->mOrderType = 'count'; - } - if( $this->mRequest->getText( 'direction' ) == 'asc' ) { - $this->mDefaultDirection = false; - } elseif( $this->mRequest->getText( 'direction' ) == 'desc' - || $this->mOrderType == 'count' ) { - $this->mDefaultDirection = true; - } - } - function getQueryInfo() { global $wgRequest; return array( @@ -46,13 +31,16 @@ class CategoryPager extends AlphabeticPager { } function getIndexField() { - # We can't use mOrderType here, since this is called from the parent - # constructor. Hmm. - if( $this->mRequest->getText( 'order' ) == 'count' ) { - return 'cat_pages'; - } else { - return "cat_title"; - } + return array( 'abc' => 'cat_title', 'count' => 'cat_pages' ); + } + + protected function getOrderTypeMessages() { + return array( 'abc' => 'special-categories-sort-abc', + 'count' => 'special-categories-sort-count' ); + } + + protected function getDefaultDirection() { + return array( 'abc' => false, 'count' => true ); } /* Override getBody to apply LinksBatch on resultset before actually outputting anything. */ @@ -80,39 +68,6 @@ class CategoryPager extends AlphabeticPager { $wgLang->formatNum( $result->cat_pages ) ); return Xml::tags('li', null, "$titleText ($count)" ) . "\n"; } - - /** Override this to order by count */ - public function getNavigationBar() { - $nav = parent::getNavigationBar() . ' ('; - if( $this->mOrderType == 'abc' ) { - $nav .= $this->makeLink( - wfMsgHTML( 'special-categories-sort-count' ), - array( 'order' => 'count' ) - ); - } else { - $nav .= $this->makeLink( - wfMsgHTML( 'special-categories-sort-abc' ), - array( 'order' => 'abc' ) - ); - } - $nav .= ') ('; - # FIXME, these are stupid query names. "order" and "dir" are already - # used. - if( $this->mDefaultDirection ) { - # Descending - $nav .= $this->makeLink( - wfMsgHTML( 'special-categories-sort-asc' ), - array( 'direction' => 'asc' ) - ); - } else { - $nav .= $this->makeLink( - wfMsgHTML( 'special-categories-sort-desc' ), - array( 'direction' => 'desc' ) - ); - } - $nav .= ')'; - return $nav; - } } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 7cf9b0e41a..45cb2aecc2 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -567,8 +567,6 @@ XHTML id names. 'categoriespagetext' => 'The following categories contain pages or media.', 'special-categories-sort-count' => 'sort by count', 'special-categories-sort-abc' => 'sort alphabetically', -'special-categories-sort-asc' => 'ascending', -'special-categories-sort-desc' => 'descending', 'pagecategories' => '{{PLURAL:$1|Category|Categories}}', 'pagecategorieslink' => 'Special:Categories', # don't translate or duplicate this message to other languages 'category_header' => 'Pages in category "$1"', @@ -1731,6 +1729,8 @@ The [http://meta.wikimedia.org/wiki/Help:Job_queue job queue] length is '''\$7'' 'notargettext' => 'You have not specified a target page or user to perform this function on.', 'pager-newer-n' => '{{PLURAL:$1|newer 1|newer $1}}', 'pager-older-n' => '{{PLURAL:$1|older 1|older $1}}', +'pager-sort-asc' => 'ascending', +'pager-sort-desc' => 'descending', # Book sources 'booksources' => 'Book sources', diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index a6027fe7c5..ef7f956068 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -117,8 +117,6 @@ $wgMessageStructure = array( 'categoriespagetext', 'special-categories-sort-count', 'special-categories-sort-abc', - 'special-categories-sort-asc', - 'special-categories-sort-desc', 'pagecategories', 'pagecategorieslink', 'category_header', @@ -1135,6 +1133,8 @@ $wgMessageStructure = array( 'notargettext', 'pager-newer-n', 'pager-older-n', + 'pager-sort-asc', + 'pager-sort-desc', ), 'booksources' => array( 'booksources', -- 2.20.1