r44033 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r44032‎ | r44033 | r44034 >
Date:00:17, 29 November 2008
Author:aaron
Status:resolved (Comments)
Tags:
Comment:
* Use insertArticleLink() to remove duplication
* Break some long lines
Modified paths:
  • /trunk/phase3/includes/ChangesList.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ChangesList.php
@@ -24,6 +24,7 @@
2525 */
2626 class ChangesList {
2727 # Called by history lists and recent changes
 28+ public $skin;
2829
2930 /**
3031 * Changeslist contructor
@@ -45,7 +46,8 @@
4647 $sk = $user->getSkin();
4748 $list = NULL;
4849 if( wfRunHooks( 'FetchChangesList', array( &$user, &$sk, &$list ) ) ) {
49 - return $user->getOption( 'usenewrc' ) ? new EnhancedChangesList( $sk ) : new OldChangesList( $sk );
 50+ return $user->getOption( 'usenewrc' ) ?
 51+ new EnhancedChangesList( $sk ) : new OldChangesList( $sk );
5052 } else {
5153 return $list;
5254 }
@@ -147,7 +149,6 @@
148150
149151 protected function insertDateHeader( &$s, $rc_timestamp ) {
150152 global $wgLang;
151 -
152153 # Make date header if necessary
153154 $date = $wgLang->date( $rc_timestamp, true, true );
154155 if( $date != $this->lastdate ) {
@@ -210,15 +211,14 @@
211212 # RTL/LTR marker
212213 $articlelink .= $wgContLang->getDirMark();
213214
214 - wfRunHooks('ChangesListInsertArticleLink',
215 - array(&$this, &$articlelink, &$s, &$rc, $unpatrolled, $watched));
 215+ wfRunHooks( 'ChangesListInsertArticleLink',
 216+ array(&$this, &$articlelink, &$s, &$rc, $unpatrolled, $watched) );
216217
217218 $s .= " $articlelink";
218219 }
219220
220221 protected function insertTimestamp( &$s, $rc ) {
221222 global $wgLang;
222 - # Timestamp
223223 $s .= $this->message['semicolon-separator'] . $wgLang->time( $rc->mAttribs['rc_timestamp'], true, true ) . ' . . ';
224224 }
225225
@@ -336,8 +336,7 @@
337337 public function recentChangesLine( &$rc, $watched = false ) {
338338 global $wgContLang, $wgRCShowChangedSize, $wgUser;
339339
340 - $fname = 'ChangesList::recentChangesLineOld';
341 - wfProfileIn( $fname );
 340+ wfProfileIn( __METHOD__ );
342341
343342 # Extract DB fields into local scope
344343 // FIXME: Would be good to replace this extract() call with something that explicitly initializes local variables.
@@ -368,19 +367,12 @@
369368 }
370369 // Regular entries
371370 } else {
372 - wfProfileIn($fname.'-page');
373 -
374371 $this->insertDiffHist($s, $rc, $unpatrolled);
375 -
376372 # M, N, b and ! (minor, new, bot and unpatrolled)
377373 $s .= $this->recentChangesFlags( $rc_type == RC_NEW, $rc_minor, $unpatrolled, '', $rc_bot );
378374 $this->insertArticleLink($s, $rc, $unpatrolled, $watched);
379 -
380 - wfProfileOut($fname.'-page');
381375 }
382376
383 - wfProfileIn( $fname.'-rest' );
384 -
385377 $this->insertTimestamp($s,$rc);
386378
387379 if( $wgRCShowChangedSize ) {
@@ -404,9 +396,7 @@
405397
406398 $s = "<li>$s</li>\n";
407399
408 - wfProfileOut( $fname.'-rest' );
409 -
410 - wfProfileOut( $fname );
 400+ wfProfileOut( __METHOD__ );
411401 return $dateheader . $s;
412402 }
413403 }
@@ -663,7 +653,7 @@
664654 if( $namehidden ) {
665655 $r .= ' <span class="history-deleted">' . wfMsgHtml('rev-deleted-event') . '</span>';
666656 } else {
667 - $r .= $this->maybeWatchedLink( $block[0]->link, $block[0]->watched );
 657+ $this->insertArticleLink( $r, $block[0], $block[0]->unpatrolled, $block[0]->watched );
668658 }
669659
670660 $r .= $wgContLang->getDirMark();
@@ -728,7 +718,8 @@
729719 $r .= "</td></tr></table>\n";
730720
731721 # Sub-entries
732 - $r .= '<div id="mw-rc-subentries-'.$jsid.'" class="mw-changeslist-hidden"><table cellpadding="0" cellspacing="0" border="0" style="background: none">';
 722+ $r .= '<div id="mw-rc-subentries-'.$jsid.'" class="mw-changeslist-hidden">';
 723+ $r .= '<table cellpadding="0" cellspacing="0" border="0" style="background: none">';
733724 foreach( $block as $rcObj ) {
734725 # Get rc_xxxx variables
735726 // FIXME: Would be good to replace this extract() call with something that explicitly initializes local variables.
@@ -751,9 +742,10 @@
752743 } else if( !ChangesList::userCan($rcObj,Revision::DELETED_TEXT) ) {
753744 $link = '<span class="history-deleted"><tt>'.$rcObj->timestamp.'</tt></span> ';
754745 } else {
755 - $rcIdEq = ($rcObj->unpatrolled && $rc_type == RC_NEW) ? '&rcid='.$rcObj->mAttribs['rc_id'] : '';
756 -
757 - $link = '<tt>'.$this->skin->makeKnownLinkObj( $rcObj->getTitle(), $rcObj->timestamp, $curIdEq.'&'.$o.$rcIdEq ).'</tt>';
 746+ $rcIdEq = ($rcObj->unpatrolled && $rc_type == RC_NEW) ?
 747+ '&rcid='.$rcObj->mAttribs['rc_id'] : '';
 748+ $link = '<tt>'.$this->skin->makeKnownLinkObj( $rcObj->getTitle(),
 749+ $rcObj->timestamp, $curIdEq.'&'.$o.$rcIdEq ).'</tt>';
758750 if( $this->isDeleted($rcObj,Revision::DELETED_TEXT) )
759751 $link = '<span class="history-deleted">'.$link.'</span> ';
760752 }
@@ -849,16 +841,13 @@
850842 */
851843 protected function recentChangesBlockLine( $rcObj ) {
852844 global $wgContLang, $wgRCShowChangedSize;
853 -
854845 # Get rc_xxxx variables
855846 // FIXME: Would be good to replace this extract() call with something that explicitly initializes local variables.
856847 extract( $rcObj->mAttribs );
857 - $curIdEq = 'curid='.$rc_cur_id;
 848+ $curIdEq = "curid={$rc_cur_id}";
858849
859850 $r = '<table cellspacing="0" cellpadding="0" border="0" style="background: none"><tr>';
860 -
861851 $r .= '<td valign="top" style="white-space: nowrap"><tt>' . $this->spacerArrow() . '&nbsp;';
862 -
863852 # Flag and Timestamp
864853 if( $rc_type == RC_MOVE || $rc_type == RC_MOVE_OVER_REDIRECT ) {
865854 $r .= '&nbsp;&nbsp;&nbsp;&nbsp;'; // 4 flags -> 4 spaces
@@ -866,33 +855,26 @@
867856 $r .= $this->recentChangesFlags( $rc_type == RC_NEW, $rc_minor, $rcObj->unpatrolled, '&nbsp;', $rc_bot );
868857 }
869858 $r .= '&nbsp;'.$rcObj->timestamp.'&nbsp;</tt></td><td>';
870 -
871859 # Article or log link
872860 if( $rc_log_type ) {
873861 $logtitle = Title::newFromText( "Log/$rc_log_type", NS_SPECIAL );
874862 $logname = LogPage::logName( $rc_log_type );
875863 $r .= '(' . $this->skin->makeKnownLinkObj($logtitle, $logname ) . ')';
876 - } else if( !$this->userCan($rcObj,Revision::DELETED_TEXT) ) {
877 - $r .= '<span class="history-deleted">' . $rcObj->link . '</span>';
878864 } else {
879 - $r .= $this->maybeWatchedLink( $rcObj->link, $rcObj->watched );
 865+ $this->insertArticleLink( $r, $rcObj, $rcObj->unpatrolled, $rcObj->watched );
880866 }
881 -
882867 # Diff and hist links
883868 if ( $rc_type != RC_LOG ) {
884869 $r .= ' ('. $rcObj->difflink . $this->message['semicolon-separator'];
885870 $r .= $this->skin->makeKnownLinkObj( $rcObj->getTitle(), wfMsg( 'hist' ), $curIdEq.'&action=history' ) . ')';
886871 }
887872 $r .= ' . . ';
888 -
889873 # Character diff
890874 if( $wgRCShowChangedSize ) {
891875 $r .= ( $rcObj->getCharacterDifference() == '' ? '' : $rcObj->getCharacterDifference() . ' . . ' ) ;
892876 }
893 -
894877 # User/talk
895878 $r .= ' '.$rcObj->userlink . $rcObj->usertalklink;
896 -
897879 # Log action (if any)
898880 if( $rc_log_type ) {
899881 if( $this->isDeleted($rcObj,LogPage::DELETED_ACTION) ) {
@@ -902,7 +884,6 @@
903885 $this->skin, LogPage::extractParams($rc_params), true, true );
904886 }
905887 }
906 -
907888 # Edit or log comment
908889 if( $rc_type != RC_MOVE && $rc_type != RC_MOVE_OVER_REDIRECT ) {
909890 // log comment
@@ -912,7 +893,6 @@
913894 $r .= $this->skin->commentBlock( $rc_comment, $rcObj->getTitle() );
914895 }
915896 }
916 -
917897 # Show how many people are watching this if enabled
918898 $r .= $this->numberofWatchingusers($rcObj->numberofWatchingusers);
919899
@@ -936,7 +916,6 @@
937917 $blockOut .= $this->recentChangesBlockGroup( $block );
938918 }
939919 }
940 -
941920 return '<div>'.$blockOut.'</div>';
942921 }
943922

Comments

#Comment by Brion VIBBER (talk | contribs)   21:31, 11 December 2008

This causes a regression in display of log entries.

Example: expanded nodes for deletion log items in enhanced RC -- the root note should refer to deletion log, but lists the first page target instead. screen shot

#Comment by Brion VIBBER (talk | contribs)   21:55, 11 December 2008

Resolved by r44470

Status & tagging log