r92231 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92230‎ | r92231 | r92232 >
Date:08:42, 15 July 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
* Use local context instead of global variables
* Removed parameters that could be replaced by context
* Call Linker methods statically
* Removed two unused methods: countWatchlist() and showItemCount()
* The parameter of SpecialEditWatch::buildTools() is now unused
Modified paths:
  • /trunk/phase3/includes/specials/SpecialEditWatchlist.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialEditWatchlist.php
——@@ -28,32 +28,33 @@
2929 * @param $mode int
3030 */
3131 public function execute( $mode ) {
32 - global $wgUser, $wgOut, $wgRequest;
3332 if( wfReadOnly() ) {
34 - $wgOut->readOnlyPage();
 33+ throw new ReadOnlyError;
3534 return;
3635 }
3736
 37+ $out = $this->getOutput();
 38+
3839 # Anons don't get a watchlist
39 - if( $wgUser->isAnon() ) {
40 - $wgOut->setPageTitle( wfMsg( 'watchnologin' ) );
41 - $llink = $this->getSkin()->linkKnown(
 40+ if( $this->getUser()->isAnon() ) {
 41+ $out->setPageTitle( wfMsg( 'watchnologin' ) );
 42+ $llink = Linker::linkKnown(
4243 SpecialPage::getTitleFor( 'Userlogin' ),
4344 wfMsgHtml( 'loginreqlink' ),
4445 array(),
4546 array( 'returnto' => $this->getTitle()->getPrefixedText() )
4647 );
47 - $wgOut->addWikiMsgArray( 'watchlistanontext', array( $llink ), array( 'replaceafter' ) );
 48+ $out->addHTML( wfMessage( 'watchlistanontext' )->rawParam( $llink )->parse() );
4849 return;
4950 }
5051
5152 $sub = wfMsgExt(
5253 'watchlistfor2',
5354 array( 'parseinline', 'replaceafter' ),
54 - $wgUser->getName(),
55 - SpecialEditWatchlist::buildTools( $this->getSkin() )
 55+ $this->getUser()->getName(),
 56+ SpecialEditWatchlist::buildTools( null )
5657 );
57 - $wgOut->setSubtitle( $sub );
 58+ $out->setSubtitle( $sub );
5859
5960 # B/C: $mode used to be waaay down the parameter list, and the first parameter
6061 # was $wgUser
——@@ -63,7 +64,7 @@
6465 $mode = $args[3];
6566 }
6667 }
67 - $mode = self::getMode( $wgRequest, $mode );
 68+ $mode = self::getMode( $this->getRequest(), $mode );
6869
6970 switch( $mode ) {
7071 case self::EDIT_CLEAR:
——@@ -71,21 +72,21 @@
7273 // Pass on to the raw editor, from which it's very easy to clear.
7374
7475 case self::EDIT_RAW:
75 - $wgOut->setPageTitle( wfMsg( 'watchlistedit-raw-title' ) );
76 - $form = $this->getRawForm( $wgUser );
 76+ $out->setPageTitle( wfMsg( 'watchlistedit-raw-title' ) );
 77+ $form = $this->getRawForm();
7778 if( $form->show() ){
78 - $wgOut->addHTML( $this->successMessage );
79 - $wgOut->returnToMain();
 79+ $out->addHTML( $this->successMessage );
 80+ $out->returnToMain();
8081 }
8182 break;
8283
8384 case self::EDIT_NORMAL:
8485 default:
85 - $wgOut->setPageTitle( wfMsg( 'watchlistedit-normal-title' ) );
86 - $form = $this->getNormalForm( $wgUser );
 86+ $out->setPageTitle( wfMsg( 'watchlistedit-normal-title' ) );
 87+ $form = $this->getNormalForm();
8788 if( $form->show() ){
88 - $wgOut->addHTML( $this->successMessage );
89 - $wgOut->returnToMain();
 89+ $out->addHTML( $this->successMessage );
 90+ $out->returnToMain();
9091 }
9192 break;
9293 }
——@@ -117,16 +118,15 @@
118119 }
119120
120121 public function submitRaw( $data ){
121 - global $wgUser, $wgLang;
122122 $wanted = $this->extractTitles( $data['Titles'] );
123 - $current = $this->getWatchlist( $wgUser );
 123+ $current = $this->getWatchlist();
124124
125125 if( count( $wanted ) > 0 ) {
126126 $toWatch = array_diff( $wanted, $current );
127127 $toUnwatch = array_diff( $current, $wanted );
128 - $this->watchTitles( $toWatch, $wgUser );
129 - $this->unwatchTitles( $toUnwatch, $wgUser );
130 - $wgUser->invalidateCache();
 128+ $this->watchTitles( $toWatch );
 129+ $this->unwatchTitles( $toUnwatch );
 130+ $this->getUser()->invalidateCache();
131131
132132 if( count( $toWatch ) > 0 || count( $toUnwatch ) > 0 ){
133133 $this->successMessage = wfMessage( 'watchlistedit-raw-done' )->parse();
——@@ -137,26 +137,26 @@
138138 if( count( $toWatch ) > 0 ) {
139139 $this->successMessage .= wfMessage(
140140 'watchlistedit-raw-added',
141 - $wgLang->formatNum( count( $toWatch ) )
 141+ $this->getLang()->formatNum( count( $toWatch ) )
142142 );
143 - $this->showTitles( $toWatch, $this->successMessage, $this->getSkin() );
 143+ $this->showTitles( $toWatch, $this->successMessage );
144144 }
145145
146146 if( count( $toUnwatch ) > 0 ) {
147147 $this->successMessage .= wfMessage(
148148 'watchlistedit-raw-removed',
149 - $wgLang->formatNum( count( $toUnwatch ) )
 149+ $this->getLang()->formatNum( count( $toUnwatch ) )
150150 );
151 - $this->showTitles( $toUnwatch, $this->successMessage, $this->getSkin() );
 151+ $this->showTitles( $toUnwatch, $this->successMessage );
152152 }
153153 } else {
154 - $this->clearWatchlist( $wgUser );
155 - $wgUser->invalidateCache();
 154+ $this->clearWatchlist();
 155+ $this->getLang()->invalidateCache();
156156 $this->successMessage .= wfMessage(
157157 'watchlistedit-raw-removed',
158 - $wgLang->formatNum( count( $current ) )
 158+ $this->getLang()->formatNum( count( $current ) )
159159 );
160 - $this->showTitles( $current, $this->successMessage, $this->getSkin() );
 160+ $this->showTitles( $current, $this->successMessage );
161161 }
162162 return true;
163163 }
——@@ -169,9 +169,8 @@
170170 *
171171 * @param $titles array of strings, or Title objects
172172 * @param $output String
173 - * @param $skin Skin
174173 */
175 - private function showTitles( $titles, &$output, $skin ) {
 174+ private function showTitles( $titles, &$output ) {
176175 $talk = wfMsgHtml( 'talkpagelinktext' );
177176 // Do a batch existence check
178177 $batch = new LinkBatch();
——@@ -193,8 +192,8 @@
194193 }
195194 if( $title instanceof Title ) {
196195 $output .= "<li>"
197 - . $skin->link( $title )
198 - . ' (' . $skin->link( $title->getTalkPage(), $talk )
 196+ . Linker::link( $title )
 197+ . ' (' . Linker::link( $title->getTalkPage(), $talk )
199198 . ")</li>\n";
200199 }
201200 }
——@@ -202,33 +201,19 @@
203202 }
204203
205204 /**
206 - * Count the number of titles on a user's watchlist, excluding talk pages
207 - *
208 - * @param $user User
209 - * @return int
210 - */
211 - private function countWatchlist( $user ) {
212 - $dbr = wfGetDB( DB_MASTER );
213 - $res = $dbr->select( 'watchlist', 'COUNT(*) AS count', array( 'wl_user' => $user->getId() ), __METHOD__ );
214 - $row = $dbr->fetchObject( $res );
215 - return ceil( $row->count / 2 ); // Paranoia
216 - }
217 -
218 - /**
219205 * Prepare a list of titles on a user's watchlist (excluding talk pages)
220206 * and return an array of (prefixed) strings
221207 *
222 - * @param $user User
223208 * @return array
224209 */
225 - private function getWatchlist( $user ) {
 210+ private function getWatchlist() {
226211 $list = array();
227212 $dbr = wfGetDB( DB_MASTER );
228213 $res = $dbr->select(
229214 'watchlist',
230215 '*',
231216 array(
232 - 'wl_user' => $user->getId(),
 217+ 'wl_user' => $this->getUser()->getId(),
233218 ),
234219 __METHOD__
235220 );
——@@ -248,10 +233,9 @@
249234 * and return as a two-dimensional array with namespace, title and
250235 * redirect status
251236 *
252 - * @param $user User
253237 * @return array
254238 */
255 - private function getWatchlistInfo( $user ) {
 239+ private function getWatchlistInfo() {
256240 $titles = array();
257241 $dbr = wfGetDB( DB_MASTER );
258242
——@@ -265,7 +249,7 @@
266250 'page_is_redirect',
267251 'page_latest'
268252 ),
269 - array( 'wl_user' => $user->getId() ),
 253+ array( 'wl_user' => $this->getUser()->getId() ),
270254 __METHOD__,
271255 array( 'ORDER BY' => 'wl_namespace, wl_title' ),
272256 array( 'page' => array(
——@@ -296,33 +280,13 @@
297281 }
298282
299283 /**
300 - * Show a message indicating the number of items on the user's watchlist,
301 - * and return this count for additional checking
302 - *
303 - * @param $output OutputPage
304 - * @param $user User
305 - * @return int
306 - */
307 - private function showItemCount( $output, $user ) {
308 - if( ( $count = $this->countWatchlist( $user ) ) > 0 ) {
309 - $output->addHTML( wfMsgExt( 'watchlistedit-numitems', 'parse',
310 - $GLOBALS['wgLang']->formatNum( $count ) ) );
311 - } else {
312 - $output->addHTML( wfMsgExt( 'watchlistedit-noitems', 'parse' ) );
313 - }
314 - return $count;
315 - }
316 -
317 - /**
318284 * Remove all titles from a user's watchlist
319 - *
320 - * @param $user User
321285 */
322 - private function clearWatchlist( $user ) {
 286+ private function clearWatchlist() {
323287 $dbw = wfGetDB( DB_MASTER );
324288 $dbw->delete(
325289 'watchlist',
326 - array( 'wl_user' => $user->getId() ),
 290+ array( 'wl_user' => $this->getUser()->getId() ),
327291 __METHOD__
328292 );
329293 }
——@@ -334,9 +298,8 @@
335299 * is preferred, since Titles are very memory-heavy
336300 *
337301 * @param $titles Array of strings, or Title objects
338 - * @param $user User
339302 */
340 - private function watchTitles( $titles, $user ) {
 303+ private function watchTitles( $titles ) {
341304 $dbw = wfGetDB( DB_MASTER );
342305 $rows = array();
343306 foreach( $titles as $title ) {
——@@ -345,13 +308,13 @@
346309 }
347310 if( $title instanceof Title ) {
348311 $rows[] = array(
349 - 'wl_user' => $user->getId(),
 312+ 'wl_user' => $this->getUser()->getId(),
350313 'wl_namespace' => ( $title->getNamespace() & ~1 ),
351314 'wl_title' => $title->getDBkey(),
352315 'wl_notificationtimestamp' => null,
353316 );
354317 $rows[] = array(
355 - 'wl_user' => $user->getId(),
 318+ 'wl_user' => $this->getUser()->getId(),
356319 'wl_namespace' => ( $title->getNamespace() | 1 ),
357320 'wl_title' => $title->getDBkey(),
358321 'wl_notificationtimestamp' => null,
——@@ -368,9 +331,8 @@
369332 * is preferred, since Titles are very memory-heavy
370333 *
371334 * @param $titles Array of strings, or Title objects
372 - * @param $user User
373335 */
374 - private function unwatchTitles( $titles, $user ) {
 336+ private function unwatchTitles( $titles ) {
375337 $dbw = wfGetDB( DB_MASTER );
376338 foreach( $titles as $title ) {
377339 if( !$title instanceof Title ) {
——@@ -380,7 +342,7 @@
381343 $dbw->delete(
382344 'watchlist',
383345 array(
384 - 'wl_user' => $user->getId(),
 346+ 'wl_user' => $this->getUser()->getId(),
385347 'wl_namespace' => ( $title->getNamespace() & ~1 ),
386348 'wl_title' => $title->getDBkey(),
387349 ),
——@@ -389,34 +351,32 @@
390352 $dbw->delete(
391353 'watchlist',
392354 array(
393 - 'wl_user' => $user->getId(),
 355+ 'wl_user' => $this->getUser()->getId(),
394356 'wl_namespace' => ( $title->getNamespace() | 1 ),
395357 'wl_title' => $title->getDBkey(),
396358 ),
397359 __METHOD__
398360 );
399 - $article = new Article($title);
400 - wfRunHooks('UnwatchArticleComplete',array(&$user,&$article));
 361+ $article = new Article( $title, 0 );
 362+ wfRunHooks( 'UnwatchArticleComplete', array( $this->getUser(), &$article ) );
401363 }
402364 }
403365 }
404366
405367 public function submitNormal( $data ) {
406 - global $wgUser;
407368 $removed = array();
408369
409370 foreach( $data as $titles ) {
410 - $this->unwatchTitles( $titles, $wgUser );
 371+ $this->unwatchTitles( $titles );
411372 $removed += $titles;
412373 }
413374
414375 if( count( $removed ) > 0 ) {
415 - global $wgLang;
416376 $this->successMessage = wfMessage(
417377 'watchlistedit-normal-done',
418 - $wgLang->formatNum( count( $removed ) )
 378+ $this->getLang()->formatNum( count( $removed ) )
419379 );
420 - $this->showTitles( $removed, $this->successMessage, $this->getSkin() );
 380+ $this->showTitles( $removed, $this->successMessage );
421381 return true;
422382 } else {
423383 return false;
——@@ -426,15 +386,14 @@
427387 /**
428388 * Get the standard watchlist editing form
429389 *
430 - * @param $user User
431390 * @return HTMLForm
432391 */
433 - protected function getNormalForm( $user ){
 392+ protected function getNormalForm(){
434393 global $wgContLang;
435 - $skin = $user->getSkin();
 394+
436395 $fields = array();
437396
438 - foreach( $this->getWatchlistInfo( $user ) as $namespace => $pages ){
 397+ foreach( $this->getWatchlistInfo() as $namespace => $pages ){
439398
440399 $namespace == NS_MAIN
441400 ? wfMsgHtml( 'blanknamespace' )
——@@ -448,7 +407,7 @@
449408
450409 foreach( $pages as $dbkey => $redirect ){
451410 $title = Title::makeTitleSafe( $namespace, $dbkey );
452 - $text = $this->buildRemoveLine( $title, $redirect, $skin );
 411+ $text = $this->buildRemoveLine( $title, $redirect );
453412 $fields['TitlesNs'.$namespace]['options'][$text] = $title->getEscapedText();
454413 }
455414 }
——@@ -467,49 +426,41 @@
468427 *
469428 * @param $title Title
470429 * @param $redirect bool
471 - * @param $skin Skin
472430 * @return string
473431 */
474 - private function buildRemoveLine( $title, $redirect, $skin ) {
475 - global $wgLang;
476 -
477 - $link = $skin->link( $title );
 432+ private function buildRemoveLine( $title, $redirect ) {
 433+ $link = Linker::link( $title );
478434 if( $redirect ) {
479435 $link = '<span class="watchlistredir">' . $link . '</span>';
480436 }
481 - $tools[] = $skin->link( $title->getTalkPage(), wfMsgHtml( 'talkpagelinktext' ) );
 437+ $tools[] = Linker::link( $title->getTalkPage(), wfMsgHtml( 'talkpagelinktext' ) );
482438 if( $title->exists() ) {
483 - $tools[] = $skin->link(
 439+ $tools[] = Linker::linkKnown(
484440 $title,
485441 wfMsgHtml( 'history_short' ),
486442 array(),
487 - array( 'action' => 'history' ),
488 - array( 'known', 'noclasses' )
 443+ array( 'action' => 'history' )
489444 );
490445 }
491446 if( $title->getNamespace() == NS_USER && !$title->isSubpage() ) {
492 - $tools[] = $skin->link(
 447+ $tools[] = Linker::linkKnown(
493448 SpecialPage::getTitleFor( 'Contributions', $title->getText() ),
494 - wfMsgHtml( 'contributions' ),
495 - array(),
496 - array(),
497 - array( 'known', 'noclasses' )
 449+ wfMsgHtml( 'contributions' )
498450 );
499451 }
500452
501 - wfRunHooks( 'WatchlistEditorBuildRemoveLine', array( &$tools, $title, $redirect, $skin ) );
 453+ wfRunHooks( 'WatchlistEditorBuildRemoveLine', array( &$tools, $title, $redirect, $this->getSkin() ) );
502454
503 - return $link . " (" . $wgLang->pipeList( $tools ) . ")";
 455+ return $link . " (" . $this->getLang()->pipeList( $tools ) . ")";
504456 }
505457
506458 /**
507459 * Get a form for editing the watchlist in "raw" mode
508460 *
509 - * @param $user User
510461 * @return HTMLForm
511462 */
512 - protected function getRawForm( $user ){
513 - $titles = implode( array_map( 'htmlspecialchars', $this->getWatchlist( $user ) ), "\n" );
 463+ protected function getRawForm(){
 464+ $titles = implode( array_map( 'htmlspecialchars', $this->getWatchlist() ), "\n" );
514465 $fields = array(
515466 'Titles' => array(
516467 'type' => 'textarea',
——@@ -560,10 +511,10 @@
561512 * Build a set of links for convenient navigation
562513 * between watchlist viewing and editing modes
563514 *
564 - * @param $skin Skin to use
 515+ * @param $unused Unused
565516 * @return string
566517 */
567 - public static function buildTools( $skin ) {
 518+ public static function buildTools( $unused ) {
568519 global $wgLang;
569520
570521 $tools = array();
——@@ -574,7 +525,7 @@
575526 );
576527 foreach( $modes as $mode => $arr ) {
577528 // can use messages 'watchlisttools-view', 'watchlisttools-edit', 'watchlisttools-raw'
578 - $tools[] = $skin->linkKnown(
 529+ $tools[] = Linker::linkKnown(
579530 SpecialPage::getTitleFor( $arr[0], $arr[1] ),
580531 wfMsgHtml( "watchlisttools-{$mode}" )
581532 );
——@@ -593,10 +544,9 @@
594545 */
595546 class EditWatchlistNormalHTMLForm extends HTMLForm {
596547 public function getLegend( $namespace ){
597 - global $wgLang;
598548 $namespace = substr( $namespace, 2 );
599549 return $namespace == NS_MAIN
600550 ? wfMsgHtml( 'blanknamespace' )
601 - : htmlspecialchars( $wgLang->getFormattedNsText( $namespace ) );
 551+ : htmlspecialchars( $this->getContext()->getLang()->getFormattedNsText( $namespace ) );
602552 }
603553 }

Sign-offs

UserFlagDate
Happy-meloninspected21:43, 28 August 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r92242Per Nikerabbit, follow-up to r92231: removed unreachable codeialex14:28, 15 July 2011
r93385Fu r92231: PHP Fatal error: Call to a member function parse() on a non-objec...nikerabbit12:13, 28 July 2011
r95608Per Nikerabbit, fix for r92231: this was the wrong objectialex20:59, 27 August 2011

Comments

#Comment by Nikerabbit (talk | contribs)   11:25, 15 July 2011 
-			$wgOut->readOnlyPage();
+			throw new ReadOnlyError;
 			return;

The return is now unreachable.

Content language is not part of the context?

#Comment by IAlex (talk | contribs)   14:28, 15 July 2011

Removed in r92242.

Content language only depends on the wiki and not on the request itself, that's why it's not in the context object (as $wgParser and some others).

#Comment by Nikerabbit (talk | contribs)   08:19, 27 August 2011 
-                       $wgUser->invalidateCache();
+                       $this->clearWatchlist();
+                       $this->getLang()->invalidateCache();
#Comment by IAlex (talk | contribs)   21:00, 27 August 2011

Fixed in r95608.

Status & tagging log