r43977 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r43976‎ | r43977 | r43978 >
Date:12:28, 26 November 2008
Author:werdna
Status:ok (Comments)
Tags:
Comment:
Fix display of old revisions - show the newest 10, not the oldest 10 :)
Modified paths:
  • /trunk/extensions/Configure/Configure.page.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Configure/Configure.page.php
——@@ -345,9 +345,7 @@
346346 ksort($versions); ## Put in ascending order for now.
347347
348348 foreach ( $versions as $ts => $data ) {
349 - if ( $count > 10 ) {
350 - break;
351 - } elseif ( in_array( $this->mWiki, $wgConf->getWikisInVersion( $ts ) ) ) {
 349+ if ( in_array( $this->mWiki, $wgConf->getWikisInVersion( $ts ) ) ) {
352350 $count++;
353351 $link = $skin->makeKnownLinkObj( $title, $wgLang->timeAndDate( $ts ), "version=$ts" );
354352 $diffLink = '';
——@@ -377,6 +375,8 @@
378376
379377 ## Reset into descending order
380378 $links = array_reverse( $links );
 379+ ## Take out the first ten...
 380+ $links = array_slice( $links, 0, 10 );
381381
382382 $text = '<fieldset><legend>' . wfMsgHtml( 'configure-old' ) . '</legend>';
383383 if ( !count( $links ) ) {

Comments

#Comment by Aaron Schulz (talk | contribs)   23:04, 26 November 2008

Why does this need ksort and array_reverse?

#Comment by Werdna (talk | contribs)   23:31, 26 November 2008

It needs to be sorted, as the list comes from either a database SELECT or a file glob(), neither of which are guaranteed to give a particular order (although it happens that with the database we use, with the indices we use, it happens to come back in ascending order). We want it in descending order, hence the array_reverse

#Comment by Aaron Schulz (talk | contribs)   12:40, 27 November 2008

Yes, but why does it need *both*?

#Comment by Werdna (talk | contribs)   12:42, 27 November 2008

Because ksort sorts it in ascending order. We want it in descending order. Also, we want it in ascending order for the loop, because we store the ts of the last revision read and feed it forward to show diffs.

#Comment by Aaron Schulz (talk | contribs)   13:25, 27 November 2008

What I'm getting at is if there is a way to use krsort()

#Comment by Werdna (talk | contribs)   13:27, 27 November 2008

No, because I need to pass the timestamp forward to the next diff so we can have diff links.

We *could* use krsort, but really, two quicksorts of a few hundred items (max) every few hundred thousand requests do not a performance issue make.

#Comment by Aaron Schulz (talk | contribs)   13:52, 27 November 2008

If it is not *easy* to improve, then it doesn't matter.

Status & tagging log