r91842 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91841‎ | r91842 | r91843 >
Date:18:12, 10 July 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
* Changed dynamic calls to Linker methods into static ones
* Also preferably pass a Language object instead of a Skin one or boolean indicating whether it's for content. No other calls to these methods in core or extensions.
Modified paths:
  • /trunk/phase3/includes/LogEventsList.php (modified) (history)
  • /trunk/phase3/includes/LogPage.php (modified) (history)
  • /trunk/phase3/includes/PatrolLog.php (modified) (history)
  • /trunk/phase3/includes/revisiondelete/RevisionDeleter.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/PatrolLog.php
——@@ -35,16 +35,14 @@
3636 *
3737 * @param $title Title of the page that was patrolled
3838 * @param $params Array: log parameters (from logging.log_params)
39 - * @param $skin Skin to use for building links, etc.
 39+ * @param $lang Language object to use, or false
4040 * @return String
4141 */
42 - public static function makeActionText( $title, $params, $skin ) {
43 - global $wgLang;
44 -
 42+ public static function makeActionText( $title, $params, $lang ) {
4543 list( $cur, /* $prev */, $auto ) = $params;
46 - if( is_object( $skin ) ) {
 44+ if( is_object( $lang ) ) {
4745 # Standard link to the page in question
48 - $link = $skin->link( $title );
 46+ $link = Linker::link( $title );
4947 if( $title->exists() ) {
5048 # Generate a diff link
5149 $query = array(
——@@ -52,9 +50,9 @@
5351 'diff' => 'prev'
5452 );
5553
56 - $diff = $skin->link(
 54+ $diff = Linker::link(
5755 $title,
58 - htmlspecialchars( wfMsg( 'patrol-log-diff', $wgLang->formatNum( $cur, true ) ) ),
 56+ htmlspecialchars( wfMsg( 'patrol-log-diff', $lang->formatNum( $cur, true ) ) ),
5957 array(),
6058 $query,
6159 array( 'known', 'noclasses' )
Index: trunk/phase3/includes/revisiondelete/RevisionDeleter.php
——@@ -67,45 +67,36 @@
6868 * @param $count Integer: The number of effected revisions.
6969 * @param $nbitfield Integer: The new bitfield for the revision.
7070 * @param $obitfield Integer: The old bitfield for the revision.
 71+ * @param $language Language object to use
7172 * @param $isForLog Boolean
72 - * @param $forContent Boolean
7373 */
74 - public static function getLogMessage( $count, $nbitfield, $obitfield, $isForLog = false, $forContent = false ) {
75 - global $wgLang, $wgContLang;
76 -
77 - $lang = $forContent ? $wgContLang : $wgLang;
78 - $msgFunc = $forContent ? "wfMsgForContent" : "wfMsg";
79 -
 74+ public static function getLogMessage( $count, $nbitfield, $obitfield, $language, $isForLog = false ) {
8075 $changes = self::getChanges( $nbitfield, $obitfield );
81 - array_walk( $changes, array( __CLASS__, 'expandMessageArray' ), $forContent );
 76+ array_walk( $changes, array( __CLASS__, 'expandMessageArray' ), $language );
8277
8378 $changesText = array();
8479
8580 if( count( $changes[0] ) ) {
86 - $changesText[] = $msgFunc( 'revdelete-hid', $lang->commaList( $changes[0] ) );
 81+ $changesText[] = wfMsgExt( 'revdelete-hid', array( 'parsemag', 'language' => $language ), $language->commaList( $changes[0] ) );
8782 }
8883 if( count( $changes[1] ) ) {
89 - $changesText[] = $msgFunc( 'revdelete-unhid', $lang->commaList( $changes[1] ) );
 84+ $changesText[] = wfMsgExt( 'revdelete-unhid', array( 'parsemag', 'language' => $language ), $language->commaList( $changes[1] ) );
9085 }
9186
92 - $s = $lang->semicolonList( $changesText );
 87+ $s = $language->semicolonList( $changesText );
9388 if( count( $changes[2] ) ) {
9489 $s .= $s ? ' (' . $changes[2][0] . ')' : ' ' . $changes[2][0];
9590 }
9691
9792 $msg = $isForLog ? 'logdelete-log-message' : 'revdelete-log-message';
98 - return wfMsgExt( $msg, $forContent ? array( 'parsemag', 'content' ) : array( 'parsemag' ), $s, $lang->formatNum($count) );
 93+ return wfMsgExt( $msg, array( 'parsemag', 'language' => $language ), $s, $language->formatNum($count) );
9994 }
10095
101 - private static function expandMessageArray(& $msg, $key, $forContent) {
 96+ private static function expandMessageArray( &$msg, $key, $language ) {
10297 if ( is_array ( $msg ) ) {
103 - array_walk( $msg, array( __CLASS__, 'expandMessageArray' ), $forContent );
 98+ array_walk( $msg, array( __CLASS__, 'expandMessageArray' ), $language );
10499 } else {
105 - if ( $forContent ) {
106 - $msg = wfMsgForContent($msg);
107 - } else {
108 - $msg = wfMsg($msg);
109 - }
 100+ $msg = wfMsgExt( $msg, array( 'parsemag', 'language' => $language ) );
110101 }
111102 }
112103
Index: trunk/phase3/includes/LogEventsList.php
——@@ -169,7 +169,7 @@
170170 $hideVal = 1 - intval($val);
171171 $query[$queryKey] = $hideVal;
172172
173 - $link = $this->skin->link(
 173+ $link = Linker::link(
174174 $this->getDisplayTitle(),
175175 $messages[$hideVal],
176176 array(),
——@@ -352,10 +352,10 @@
353353 $userLinks = '<span class="history-deleted">' .
354354 wfMsgHtml( 'rev-deleted-user' ) . '</span>';
355355 } else {
356 - $userLinks = $this->skin->userLink( $row->log_user, $row->user_name );
 356+ $userLinks = Linker::userLink( $row->log_user, $row->user_name );
357357 // Talk|Contribs links...
358358 if( !( $this->flags & self::NO_EXTRA_USER_LINKS ) ) {
359 - $userLinks .= $this->skin->userToolLinks(
 359+ $userLinks .= Linker::userToolLinks(
360360 $row->log_user, $row->user_name, true, 0, $row->user_editcount );
361361 }
362362 }
——@@ -380,7 +380,7 @@
381381 } else {
382382 global $wgLang;
383383 $comment = $wgLang->getDirMark() .
384 - $this->skin->commentBlock( $row->log_comment );
 384+ Linker::commentBlock( $row->log_comment );
385385 }
386386 return $comment;
387387 }
——@@ -405,7 +405,7 @@
406406 if( self::typeAction( $row, 'move', 'move', 'move' ) && !empty( $paramArray[0] ) ) {
407407 $destTitle = Title::newFromText( $paramArray[0] );
408408 if( $destTitle ) {
409 - $revert = '(' . $this->skin->link(
 409+ $revert = '(' . Linker::link(
410410 SpecialPage::getTitleFor( 'Movepage' ),
411411 $this->message['revertmove'],
412412 array(),
——@@ -425,7 +425,7 @@
426426 } else {
427427 $viewdeleted = $this->message['undeletelink'];
428428 }
429 - $revert = '(' . $this->skin->link(
 429+ $revert = '(' . Linker::link(
430430 SpecialPage::getTitleFor( 'Undelete' ),
431431 $viewdeleted,
432432 array(),
——@@ -435,7 +435,7 @@
436436 // Show unblock/change block link
437437 } elseif( self::typeAction( $row, array( 'block', 'suppress' ), array( 'block', 'reblock' ), 'block' ) ) {
438438 $revert = '(' .
439 - $this->skin->link(
 439+ Linker::link(
440440 SpecialPage::getTitleFor( 'Unblock', $row->log_title ),
441441 $this->message['unblocklink'],
442442 array(),
——@@ -443,7 +443,7 @@
444444 'known'
445445 ) .
446446 $this->message['pipe-separator'] .
447 - $this->skin->link(
 447+ Linker::link(
448448 SpecialPage::getTitleFor( 'Block', $row->log_title ),
449449 $this->message['change-blocklink'],
450450 array(),
——@@ -454,7 +454,7 @@
455455 // Show change protection link
456456 } elseif( self::typeAction( $row, 'protect', array( 'modify', 'protect', 'unprotect' ) ) ) {
457457 $revert .= ' (' .
458 - $this->skin->link( $title,
 458+ Linker::link( $title,
459459 $this->message['hist'],
460460 array(),
461461 array(
——@@ -464,7 +464,7 @@
465465 );
466466 if( $wgUser->isAllowed( 'protect' ) ) {
467467 $revert .= $this->message['pipe-separator'] .
468 - $this->skin->link( $title,
 468+ Linker::link( $title,
469469 $this->message['protect_change'],
470470 array(),
471471 array( 'action' => 'protect' ),
——@@ -473,7 +473,7 @@
474474 $revert .= ')';
475475 // Show unmerge link
476476 } elseif( self::typeAction( $row, 'merge', 'merge', 'mergehistory' ) ) {
477 - $revert = '(' . $this->skin->link(
 477+ $revert = '(' . Linker::link(
478478 SpecialPage::getTitleFor( 'MergeHistory' ),
479479 $this->message['revertmerge'],
480480 array(),
——@@ -495,7 +495,7 @@
496496 // $paramArray[1] is a CSV of the IDs
497497 $query = $paramArray[0];
498498 // Link to each hidden object ID, $paramArray[1] is the url param
499 - $revert = '(' . $this->skin->link(
 499+ $revert = '(' . Linker::link(
500500 $revdel,
501501 $this->message['revdel-restore'],
502502 array(),
——@@ -510,10 +510,10 @@
511511 // Self-created users
512512 } elseif( self::typeAction( $row, 'newusers', 'create2' ) ) {
513513 if( isset( $paramArray[0] ) ) {
514 - $revert = $this->skin->userToolLinks( $paramArray[0], $title->getDBkey(), true );
 514+ $revert = Linker::userToolLinks( $paramArray[0], $title->getDBkey(), true );
515515 } else {
516516 # Fall back to a blue contributions link
517 - $revert = $this->skin->userToolLinks( 1, $title->getDBkey() );
 517+ $revert = Linker::userToolLinks( 1, $title->getDBkey() );
518518 }
519519 if( wfTimestamp( TS_MW, $row->log_timestamp ) < '20080129000000' ) {
520520 # Suppress $comment from old entries (before 2008-01-29),
——@@ -548,7 +548,7 @@
549549 $canHide = $wgUser->isAllowed( 'deleterevision' );
550550 // If event was hidden from sysops
551551 if( !self::userCan( $row, LogPage::DELETED_RESTRICTED ) ) {
552 - $del = $this->skin->revDeleteLinkDisabled( $canHide );
 552+ $del = Linker::revDeleteLinkDisabled( $canHide );
553553 } else {
554554 $target = SpecialPage::getTitleFor( 'Log', $row->log_type );
555555 $query = array(
——@@ -556,7 +556,7 @@
557557 'type' => 'logging',
558558 'ids' => $row->log_id,
559559 );
560 - $del = $this->skin->revDeleteLink( $query,
 560+ $del = Linker::revDeleteLink( $query,
561561 self::isDeleted( $row, LogPage::DELETED_RESTRICTED ), $canHide );
562562 }
563563 }
——@@ -716,7 +716,7 @@
717717 # If there is exactly one log type, we can link to Special:Log?type=foo
718718 if ( count( $types ) == 1 )
719719 $urlParam['type'] = $types[0];
720 - $s .= $wgUser->getSkin()->link(
 720+ $s .= Linker::link(
721721 SpecialPage::getTitleFor( 'Log' ),
722722 wfMsgHtml( 'log-fulllog' ),
723723 array(),
Index: trunk/phase3/includes/LogPage.php
——@@ -207,26 +207,32 @@
208208 {
209209 global $wgLang, $wgContLang, $wgLogActions;
210210
 211+ if ( is_null( $skin ) ) {
 212+ $langObj = $wgContLang;
 213+ $langObjOrNull = null;
 214+ } else {
 215+ $langObj = $wgLang;
 216+ $langObjOrNull = $wgLang;
 217+ }
 218+
211219 $key = "$type/$action";
212220 # Defer patrol log to PatrolLog class
213221 if( $key == 'patrol/patrol' ) {
214 - return PatrolLog::makeActionText( $title, $params, $skin );
 222+ return PatrolLog::makeActionText( $title, $params, $langObjOrNull );
215223 }
216224 if( isset( $wgLogActions[$key] ) ) {
217225 if( is_null( $title ) ) {
218 - $rv = wfMsgHtml( $wgLogActions[$key] );
 226+ $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'language' => $langObj ) );
219227 } else {
220 - $titleLink = self::getTitleLink( $type, $skin, $title, $params );
 228+ $titleLink = self::getTitleLink( $type, $langObjOrNull, $title, $params );
221229 if( preg_match( '/^rights\/(rights|autopromote)/', $key ) ) {
 230+ $rightsnone = wfMsgExt( 'rightsnone', array( 'parsemag', 'language' => $langObj ) );
222231 if( $skin ) {
223 - $rightsnone = wfMsg( 'rightsnone' );
224232 foreach ( $params as &$param ) {
225233 $groupArray = array_map( 'trim', explode( ',', $param ) );
226234 $groupArray = array_map( array( 'User', 'getGroupMember' ), $groupArray );
227235 $param = $wgLang->listToText( $groupArray );
228236 }
229 - } else {
230 - $rightsnone = wfMsgForContent( 'rightsnone' );
231237 }
232238 if( !isset( $params[0] ) || trim( $params[0] ) == '' ) {
233239 $params[0] = $rightsnone;
——@@ -236,11 +242,7 @@
237243 }
238244 }
239245 if( count( $params ) == 0 ) {
240 - if ( $skin ) {
241 - $rv = wfMsgHtml( $wgLogActions[$key], $titleLink );
242 - } else {
243 - $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'replaceafter', 'content' ), $titleLink );
244 - }
 246+ $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'replaceafter', 'language' => $langObj ), $titleLink );
245247 } else {
246248 $details = '';
247249 array_unshift( $params, $titleLink );
——@@ -253,7 +255,7 @@
254256 $params[1] = $wgContLang->translateBlockExpiry( $params[1] );
255257 }
256258 $params[2] = isset( $params[2] ) ?
257 - self::formatBlockFlags( $params[2], is_null( $skin ) ) : '';
 259+ self::formatBlockFlags( $params[2], $langObj ) : '';
258260
259261 // Page protections
260262 } elseif ( $type == 'protect' && count($params) == 3 ) {
——@@ -265,21 +267,13 @@
266268 }
267269 // Cascading flag...
268270 if( $params[2] ) {
269 - if ( $skin ) {
270 - $details .= ' [' . wfMsg( 'protect-summary-cascade' ) . ']';
271 - } else {
272 - $details .= ' [' . wfMsgForContent( 'protect-summary-cascade' ) . ']';
273 - }
 271+ $details .= ' [' . wfMsgExt( 'protect-summary-cascade', array( 'parsemag', 'language' => $langObj ) ) . ']';
274272 }
275273
276274 // Page moves
277275 } elseif ( $type == 'move' && count( $params ) == 3 ) {
278276 if( $params[2] ) {
279 - if ( $skin ) {
280 - $details .= ' [' . wfMsg( 'move-redirect-suppressed' ) . ']';
281 - } else {
282 - $details .= ' [' . wfMsgForContent( 'move-redirect-suppressed' ) . ']';
283 - }
 277+ $details .= ' [' . wfMsgExt( 'move-redirect-suppressed', array( 'parsemag', 'language' => $langObj ) ) . ']';
284278 }
285279
286280 // Revision deletion
——@@ -287,21 +281,17 @@
288282 $count = substr_count( $params[2], ',' ) + 1; // revisions
289283 $ofield = intval( substr( $params[3], 7 ) ); // <ofield=x>
290284 $nfield = intval( substr( $params[4], 7 ) ); // <nfield=x>
291 - $details .= ': ' . RevisionDeleter::getLogMessage( $count, $nfield, $ofield, false, is_null( $skin ) );
 285+ $details .= ': ' . RevisionDeleter::getLogMessage( $count, $nfield, $ofield, $langObj, false );
292286
293287 // Log deletion
294288 } elseif ( preg_match( '/^(delete|suppress)\/event$/', $key ) && count( $params ) == 4 ) {
295289 $count = substr_count( $params[1], ',' ) + 1; // log items
296290 $ofield = intval( substr( $params[2], 7 ) ); // <ofield=x>
297291 $nfield = intval( substr( $params[3], 7 ) ); // <nfield=x>
298 - $details .= ': ' . RevisionDeleter::getLogMessage( $count, $nfield, $ofield, true, is_null( $skin ) );
 292+ $details .= ': ' . RevisionDeleter::getLogMessage( $count, $nfield, $ofield, $langObj, true );
299293 }
300294
301 - if ( $skin ) {
302 - $rv = wfMsgHtml( $wgLogActions[$key], $params ) . $details;
303 - } else {
304 - $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'replaceafter', 'content' ), $params ) . $details;
305 - }
 295+ $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'replaceafter', 'language' => $langObj ), $params ) . $details;
306296 }
307297 }
308298 } else {
——@@ -335,14 +325,14 @@
336326 /**
337327 * TODO document
338328 * @param $type String
339 - * @param $skin Skin
 329+ * @param $lang Language or null
340330 * @param $title Title
341331 * @param $params Array
342332 * @return String
343333 */
344 - protected static function getTitleLink( $type, $skin, $title, &$params ) {
345 - global $wgLang, $wgContLang, $wgUserrightsInterwikiDelimiter;
346 - if( !$skin ) {
 334+ protected static function getTitleLink( $type, $lang, $title, &$params ) {
 335+ global $wgContLang, $wgUserrightsInterwikiDelimiter;
 336+ if( !$lang ) {
347337 return $title->getPrefixedText();
348338 }
349339 switch( $type ) {
——@@ -398,7 +388,7 @@
399389 Title::newFromText( $params[0] ),
400390 htmlspecialchars( $params[0] )
401391 );
402 - $params[1] = $wgLang->timeanddate( $params[1] );
 392+ $params[1] = $lang->timeanddate( $params[1] );
403393 break;
404394 default:
405395 if( $title->getNamespace() == NS_SPECIAL ) {
——@@ -507,19 +497,16 @@
508498 * into a more readable (and translated) form
509499 *
510500 * @param $flags Flags to format
511 - * @param $forContent Whether to localize the message depending of the user
512 - * language
 501+ * @param $lang Language object to use
513502 * @return String
514503 */
515 - public static function formatBlockFlags( $flags, $forContent = false ) {
516 - global $wgLang;
517 -
 504+ public static function formatBlockFlags( $flags, $lang ) {
518505 $flags = explode( ',', trim( $flags ) );
519506 if( count( $flags ) > 0 ) {
520507 for( $i = 0; $i < count( $flags ); $i++ ) {
521 - $flags[$i] = self::formatBlockFlag( $flags[$i], $forContent );
 508+ $flags[$i] = self::formatBlockFlag( $flags[$i], $lang );
522509 }
523 - return '(' . $wgLang->commaList( $flags ) . ')';
 510+ return '(' . $lang->commaList( $flags ) . ')';
524511 } else {
525512 return '';
526513 }
——@@ -529,19 +516,15 @@
530517 * Translate a block log flag if possible
531518 *
532519 * @param $flag int Flag to translate
533 - * @param $forContent Whether to localize the message depending of the user
534 - * language
 520+ * @param $lang Language object to use
535521 * @return String
536522 */
537 - public static function formatBlockFlag( $flag, $forContent = false ) {
 523+ public static function formatBlockFlag( $flag, $lang ) {
538524 static $messages = array();
539525 if( !isset( $messages[$flag] ) ) {
540526 $messages[$flag] = htmlspecialchars( $flag ); // Fallback
541527
542 - $msg = wfMessage( 'block-log-flags-' . $flag );
543 - if ( $forContent ) {
544 - $msg->inContentLanguage();
545 - }
 528+ $msg = wfMessage( 'block-log-flags-' . $flag )->inLanguage( $lang );
546529 if ( $msg->exists() ) {
547530 $messages[$flag] = $msg->escaped();
548531 }

Comments

#Comment by Siebrand (talk | contribs)   23:04, 11 July 2011

Maybe I forgot, or maybe I triggered for the right reason. An interface was changed here, shouldn't that do to RELEASE-NOTES?

Status & tagging log