r75767 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r75766‎ | r75767 | r75768 >
Date:00:07, 1 November 2010
Author:reedy
Status:ok
Tags:
Comment:
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/ChangeTags.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/Exception.php (modified) (history)
  • /trunk/phase3/includes/Metadata.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMssql.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)
  • /trunk/phase3/includes/filerepo/ForeignAPIRepo.php (modified) (history)
  • /trunk/phase3/includes/normal/UtfNormal.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialAllpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialExport.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialNewimages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRecentchangeslinked.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialVersion.php (modified) (history)
  • /trunk/phase3/maintenance/checkSyntax.php (modified) (history)
  • /trunk/phase3/maintenance/findhooks.php (modified) (history)
  • /trunk/phase3/maintenance/importImages.inc (modified) (history)
  • /trunk/phase3/maintenance/namespaceDupes.php (modified) (history)
  • /trunk/phase3/maintenance/rebuildrecentchanges.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/checkSyntax.php
@@ -105,7 +105,8 @@
106106 if ( !$f ) {
107107 $this->error( "Can't open file $file\n", true );
108108 }
109 - while ( $path = trim( fgets( $f ) ) ) {
 109+ $path = trim( fgets( $f ) );
 110+ while ( $path ) {
110111 $this->addPath( $path );
111112 }
112113 fclose( $f );
@@ -113,6 +114,7 @@
114115 } elseif ( $this->hasOption( 'modified' ) ) {
115116 $this->output( "Retrieving list from Subversion... " );
116117 $parentDir = wfEscapeShellArg( dirname( __FILE__ ) . '/..' );
 118+ $retval = null;
117119 $output = wfShellExec( "svn status --ignore-externals $parentDir", $retval );
118120 if ( $retval ) {
119121 $this->error( "Error retrieving list from Subversion!\n", true );
Index: trunk/phase3/maintenance/rebuildrecentchanges.php
@@ -119,7 +119,8 @@
120120 "AND rev_timestamp<'{$emit}' ORDER BY rev_timestamp DESC";
121121 $sql2 = $dbw->limitResult( $sql2, 1, false );
122122 $res2 = $dbw->query( $sql2 );
123 - if ( $row = $dbw->fetchObject( $res2 ) ) {
 123+ $row = $dbw->fetchObject( $res2 );
 124+ if ( $row ) {
124125 $lastOldId = intval( $row->rev_id );
125126 # Grab the last text size if available
126127 $lastSize = !is_null( $row->rev_len ) ? intval( $row->rev_len ) : 'NULL';
Index: trunk/phase3/maintenance/importImages.inc
@@ -18,7 +18,8 @@
1919 */
2020 function findFiles( $dir, $exts ) {
2121 if ( is_dir( $dir ) ) {
22 - if ( $dhl = opendir( $dir ) ) {
 22+ $dhl = opendir( $dir );
 23+ if ( $dhl ) {
2324 $files = array();
2425 while ( ( $file = readdir( $dhl ) ) !== false ) {
2526 if ( is_file( $dir . '/' . $file ) ) {
Index: trunk/phase3/maintenance/findhooks.php
@@ -145,7 +145,8 @@
146146 */
147147 private function getHooksFromPath( $path ) {
148148 $hooks = array();
149 - if ( $dh = opendir( $path ) ) {
 149+ $dh = opendir( $path );
 150+ if ( $dh ) {
150151 while ( ( $file = readdir( $dh ) ) !== false ) {
151152 if ( filetype( $path . $file ) == 'file' ) {
152153 $hooks = array_merge( $hooks, $this->getHooksFromFile( $path . $file ) );
@@ -180,7 +181,8 @@
181182 */
182183 private function getBadHooksFromPath( $path ) {
183184 $hooks = array();
184 - if ( $dh = opendir( $path ) ) {
 185+ $dh = opendir( $path );
 186+ if ( $dh ) {
185187 while ( ( $file = readdir( $dh ) ) !== false ) {
186188 # We don't want to read this file as it contains bad calls to wfRunHooks()
187189 if ( filetype( $path . $file ) == 'file' && !$path . $file == __FILE__ ) {
Index: trunk/phase3/maintenance/namespaceDupes.php
@@ -263,11 +263,12 @@
264264 $row->title .= $suffix;
265265 $this->output( "... *** new title {$row->title}\n" );
266266 $title = Title::makeTitleSafe( $row->namespace, $row->title );
267 - if ( ! $title ) {
 267+ if ( !$title ) {
268268 $this->output( "... !!! invalid title\n" );
269269 return false;
270270 }
271 - if ( $id = $title->getArticleId() ) {
 271+ $id = $title->getArticleId();
 272+ if ( $id ) {
272273 $this->output( "... *** page exists with ID $id ***\n" );
273274 } else {
274275 break;
Index: trunk/phase3/skins/Standard.php
@@ -186,7 +186,8 @@
187187 }
188188
189189 $link = $this->mTitle->getText();
190 - if( $nstext = $wgContLang->getNsText( $tns ) ) { # add namespace if necessary
 190+ $nstext = $wgContLang->getNsText( $tns );
 191+ if( $nstext ) { # add namespace if necessary
191192 $link = $nstext . ':' . $link;
192193 }
193194
Index: trunk/phase3/skins/Vector.php
@@ -226,7 +226,8 @@
227227 $wgUser->isAllowed( 'deletedhistory' ) &&
228228 $wgUser->isAllowed( 'undelete' )
229229 ) {
230 - if( $n = $this->mTitle->isDeleted() ) {
 230+ $n = $this->mTitle->isDeleted();
 231+ if( $n ) {
231232 $undelTitle = SpecialPage::getTitleFor( 'Undelete' );
232233 $links['actions']['undelete'] = array(
233234 'class' => false,
Index: trunk/phase3/includes/diff/DifferenceEngine.php
@@ -308,8 +308,9 @@
309309 $x1 = $xoff + (int)(($numer + ($xlim-$xoff)*$chunk) / $nchunks);
310310 for ( ; $x < $x1; $x++) {
311311 $line = $flip ? $this->yv[$x] : $this->xv[$x];
312 - if (empty($ymatches[$line]))
313 - continue;
 312+ if (empty($ymatches[$line])) {
 313+ continue;
 314+ }
314315 $matches = $ymatches[$line];
315316 reset($matches);
316317 while ( list( $junk, $y ) = each( $matches ) ) {
Index: trunk/phase3/includes/Article.php
@@ -984,15 +984,18 @@
985985 wfDebug( __METHOD__ . ": showing CSS/JS source\n" );
986986 $this->showCssOrJsPage();
987987 $outputDone = true;
988 - } else if ( $rt = Title::newFromRedirectArray( $text ) ) {
989 - wfDebug( __METHOD__ . ": showing redirect=no page\n" );
990 - # Viewing a redirect page (e.g. with parameter redirect=no)
991 - # Don't append the subtitle if this was an old revision
992 - $wgOut->addHTML( $this->viewRedirect( $rt, !$wasRedirected && $this->isCurrent() ) );
993 - # Parse just to get categories, displaytitle, etc.
994 - $this->mParserOutput = $wgParser->parse( $text, $this->mTitle, $parserOptions );
995 - $wgOut->addParserOutputNoText( $this->mParserOutput );
996 - $outputDone = true;
 988+ } else {
 989+ $rt = Title::newFromRedirectArray( $text );
 990+ if ( $rt ) {
 991+ wfDebug( __METHOD__ . ": showing redirect=no page\n" );
 992+ # Viewing a redirect page (e.g. with parameter redirect=no)
 993+ # Don't append the subtitle if this was an old revision
 994+ $wgOut->addHTML( $this->viewRedirect( $rt, !$wasRedirected && $this->isCurrent() ) );
 995+ # Parse just to get categories, displaytitle, etc.
 996+ $this->mParserOutput = $wgParser->parse( $text, $this->mTitle, $parserOptions );
 997+ $wgOut->addParserOutputNoText( $this->mParserOutput );
 998+ $outputDone = true;
 999+ }
9971000 }
9981001 break;
9991002 case 4:
Index: trunk/phase3/includes/db/DatabaseMssql.php
@@ -780,7 +780,8 @@
781781 print( "Error in fieldInfo query: " . $this->getErrors() );
782782 return false;
783783 }
784 - if ( $meta = $this->fetchRow( $res ) ) {
 784+ $meta = $this->fetchRow( $res );
 785+ if ( $meta ) {
785786 return new MssqlField( $meta );
786787 }
787788 return false;
Index: trunk/phase3/includes/ChangeTags.php
@@ -180,8 +180,8 @@
181181 // Caching...
182182 global $wgMemc;
183183 $key = wfMemcKey( 'valid-tags' );
184 -
185 - if ( $tags = $wgMemc->get( $key ) ) {
 184+ $tags = $wgMemc->get( $key );
 185+ if ( $tags ) {
186186 return $tags;
187187 }
188188
Index: trunk/phase3/includes/filerepo/ForeignAPIRepo.php
@@ -201,7 +201,8 @@
202202 }
203203
204204 $key = $this->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $name );
205 - if ( $thumbUrl = $wgMemc->get($key) ) {
 205+ $thumbUrl = $wgMemc->get($key);
 206+ if ( $thumbUrl ) {
206207 wfDebug("Got thumb from local cache. $thumbUrl \n");
207208 return $thumbUrl;
208209 }
Index: trunk/phase3/includes/filerepo/File.php
@@ -906,7 +906,8 @@
907907 $retVal = array();
908908 if ( $db->numRows( $res ) ) {
909909 foreach ( $res as $row ) {
910 - if ( $titleObj = Title::newFromRow( $row ) ) {
 910+ $titleObj = Title::newFromRow( $row )
 911+ if ( $titleObj ) {
911912 $linkCache->addGoodLinkObj( $row->page_id, $titleObj, $row->page_len, $row->page_is_redirect, $row->page_latest );
912913 $retVal[] = $titleObj;
913914 }
Index: trunk/phase3/includes/EditPage.php
@@ -1867,38 +1867,40 @@
18681868 $parserOptions->setTidy( true );
18691869 $parserOutput = $wgParser->parse( $previewtext, $this->mTitle, $parserOptions );
18701870 $previewHTML = $parserOutput->mText;
1871 - } elseif ( $rt = Title::newFromRedirectArray( $this->textbox1 ) ) {
1872 - $previewHTML = $this->mArticle->viewRedirect( $rt, false );
18731871 } else {
1874 - $toparse = $this->textbox1;
 1872+ $rt = Title::newFromRedirectArray( $this->textbox1 );
 1873+ if ( $rt ) {
 1874+ $previewHTML = $this->mArticle->viewRedirect( $rt, false );
 1875+ } else {
 1876+ $toparse = $this->textbox1;
18751877
1876 - # If we're adding a comment, we need to show the
1877 - # summary as the headline
1878 - if ( $this->section == "new" && $this->summary != "" ) {
1879 - $toparse = "== {$this->summary} ==\n\n" . $toparse;
1880 - }
 1878+ # If we're adding a comment, we need to show the
 1879+ # summary as the headline
 1880+ if ( $this->section == "new" && $this->summary != "" ) {
 1881+ $toparse = "== {$this->summary} ==\n\n" . $toparse;
 1882+ }
18811883
1882 - wfRunHooks( 'EditPageGetPreviewText', array( $this, &$toparse ) );
 1884+ wfRunHooks( 'EditPageGetPreviewText', array( $this, &$toparse ) );
18831885
1884 - // Parse mediawiki messages with correct target language
1885 - if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
1886 - list( /* $unused */, $lang ) = $wgMessageCache->figureMessage( $this->mTitle->getText() );
1887 - $obj = wfGetLangObj( $lang );
1888 - $parserOptions->setTargetLanguage( $obj );
1889 - }
 1886+ // Parse mediawiki messages with correct target language
 1887+ if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
 1888+ list( /* $unused */, $lang ) = $wgMessageCache->figureMessage( $this->mTitle->getText() );
 1889+ $obj = wfGetLangObj( $lang );
 1890+ $parserOptions->setTargetLanguage( $obj );
 1891+ }
18901892
1891 -
1892 - $parserOptions->setTidy( true );
1893 - $parserOptions->enableLimitReport();
1894 - $parserOutput = $wgParser->parse( $this->mArticle->preSaveTransform( $toparse ),
 1893+ $parserOptions->setTidy( true );
 1894+ $parserOptions->enableLimitReport();
 1895+ $parserOutput = $wgParser->parse( $this->mArticle->preSaveTransform( $toparse ),
18951896 $this->mTitle, $parserOptions );
18961897
1897 - $previewHTML = $parserOutput->getText();
1898 - $this->mParserOutput = $parserOutput;
1899 - $wgOut->addParserOutputNoText( $parserOutput );
 1898+ $previewHTML = $parserOutput->getText();
 1899+ $this->mParserOutput = $parserOutput;
 1900+ $wgOut->addParserOutputNoText( $parserOutput );
19001901
1901 - if ( count( $parserOutput->getWarnings() ) ) {
1902 - $note .= "\n\n" . implode( "\n\n", $parserOutput->getWarnings() );
 1902+ if ( count( $parserOutput->getWarnings() ) ) {
 1903+ $note .= "\n\n" . implode( "\n\n", $parserOutput->getWarnings() );
 1904+ }
19031905 }
19041906 }
19051907
Index: trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -45,7 +45,8 @@
4646 if ( $ns === NS_MEDIAWIKI ) {
4747 return wfEmptyMsg( $page ) ? '' : wfMsgExt( $page, 'content' );
4848 }
49 - if ( $title = Title::newFromText( $page, $ns ) ) {
 49+ $title = Title::newFromText( $page, $ns );
 50+ if ( $title ) {
5051 if ( $title->isValidCssJsSubpage() && $revision = Revision::newFromTitle( $title ) ) {
5152 return $revision->getRawText();
5253 }
@@ -59,7 +60,8 @@
6061 $scripts = '';
6162 foreach ( $this->getPages( $context ) as $page => $options ) {
6263 if ( $options['type'] === 'script' ) {
63 - if ( $script = $this->getContent( $page, $options['ns'] ) ) {
 64+ $script = $this->getContent( $page, $options['ns'] );
 65+ if ( $script ) {
6466 $ns = MWNamespace::getCanonicalName( $options['ns'] );
6567 $scripts .= "/*$ns:$page */\n$script\n";
6668 }
@@ -74,7 +76,8 @@
7577 foreach ( $this->getPages( $context ) as $page => $options ) {
7678 if ( $options['type'] === 'style' ) {
7779 $media = isset( $options['media'] ) ? $options['media'] : 'all';
78 - if ( $style = $this->getContent( $page, $options['ns'] ) ) {
 80+ $style = $this->getContent( $page, $options['ns'] );
 81+ if ( $style ) {
7982 if ( !isset( $styles[$media] ) ) {
8083 $styles[$media] = '';
8184 }
Index: trunk/phase3/includes/Wiki.php
@@ -108,8 +108,8 @@
109109 if( $wgRequest->getVal( 'printable' ) === 'yes' ) {
110110 $wgOut->setPrintable();
111111 }
112 -
113 - if( $curid = $wgRequest->getInt( 'curid' ) ) {
 112+ $curid = $wgRequest->getInt( 'curid' );
 113+ if( $curid ) {
114114 // URLs like this are generated by RC, because rc_title isn't always accurate
115115 $ret = Title::newFromID( $curid );
116116 } elseif( $title == '' && $action != 'delete' ) {
@@ -191,7 +191,8 @@
192192
193193 // Interwiki redirects
194194 } else if( $title->getInterwiki() != '' ) {
195 - if( $rdfrom = $request->getVal( 'rdfrom' ) ) {
 195+ $rdfrom = $request->getVal( 'rdfrom' );
 196+ if( $rdfrom ) {
196197 $url = $title->getFullURL( 'rdfrom=' . urlencode( $rdfrom ) );
197198 } else {
198199 $query = $request->getValues();
Index: trunk/phase3/includes/Title.php
@@ -2794,7 +2794,8 @@
27952795 $retVal = array();
27962796 if ( $db->numRows( $res ) ) {
27972797 foreach ( $res as $row ) {
2798 - if ( $titleObj = Title::makeTitle( $row->page_namespace, $row->page_title ) ) {
 2798+ $titleObj = Title::makeTitle( $row->page_namespace, $row->page_title )
 2799+ if ( $titleObj ) {
27992800 $linkCache->addGoodLinkObj( $row->page_id, $titleObj, $row->page_len, $row->page_is_redirect, $row->page_latest );
28002801 $retVal[] = $titleObj;
28012802 }
Index: trunk/phase3/includes/SkinTemplate.php
@@ -793,7 +793,8 @@
794794 } else {
795795 //article doesn't exist or is deleted
796796 if( $wgUser->isAllowed( 'deletedhistory' ) && $wgUser->isAllowed( 'deletedtext' ) ) {
797 - if( $n = $this->mTitle->isDeleted() ) {
 797+ $n = $this->mTitle->isDeleted();
 798+ if( $n ) {
798799 $undelTitle = SpecialPage::getTitleFor( 'Undelete' );
799800 $content_actions['undelete'] = array(
800801 'class' => false,
Index: trunk/phase3/includes/specials/SpecialAllpages.php
@@ -188,7 +188,8 @@
189189 array ('LIMIT' => 2, 'OFFSET' => $maxPerSubpage - 1, 'ORDER BY' => 'page_title ASC')
190190 );
191191
192 - if( $s = $dbr->fetchObject( $res ) ) {
 192+ $s = $dbr->fetchObject( $res );
 193+ if( $s ) {
193194 array_push( $lines, $s->page_title );
194195 } else {
195196 // Final chunk, but ended prematurely. Go back and find the end.
@@ -198,7 +199,8 @@
199200 array_push( $lines, $endTitle );
200201 $done = true;
201202 }
202 - if( $s = $res->fetchObject() ) {
 203+ $s = $res->fetchObject();
 204+ if( $s ) {
203205 array_push( $lines, $s->page_title );
204206 $lastTitle = $s->page_title;
205207 } else {
Index: trunk/phase3/includes/specials/SpecialRecentchangeslinked.php
@@ -99,7 +99,8 @@
100100 $query_options = array();
101101
102102 // left join with watchlist table to highlight watched rows
103 - if( $uid = $wgUser->getId() ) {
 103+ $uid = $wgUser->getId();
 104+ if( $uid ) {
104105 $tables[] = 'watchlist';
105106 $select[] = 'wl_user';
106107 $join_conds['watchlist'] = array( 'LEFT JOIN', "wl_user={$uid} AND wl_title=rc_title AND wl_namespace=rc_namespace" );
Index: trunk/phase3/includes/specials/SpecialExport.php
@@ -229,8 +229,8 @@
230230 if( $this->templates ) {
231231 $pageSet = $this->getTemplates( $inputPages, $pageSet );
232232 }
233 -
234 - if( $linkDepth = $this->pageLinkDepth ) {
 233+ $linkDepth = $this->pageLinkDepth;
 234+ if( $linkDepth ) {
235235 $pageSet = $this->getPageLinks( $inputPages, $pageSet, $linkDepth );
236236 }
237237
Index: trunk/phase3/includes/specials/SpecialVersion.php
@@ -293,7 +293,10 @@
294294 $out .= '<tr><td colspan="4">' . $this->listToText( $wgExtensionFunctions ) . "</td></tr>\n";
295295 }
296296
297 - if ( $cnt = count( $tags = $wgParser->getTags() ) ) {
 297+ $tags = $wgParser->getTags();
 298+ $cnt = count( $tags );
 299+
 300+ if ( $cnt ) {
298301 for ( $i = 0; $i < $cnt; ++$i ) {
299302 $tags[$i] = "&lt;{$tags[$i]}&gt;";
300303 }
Index: trunk/phase3/includes/specials/SpecialNewimages.php
@@ -74,8 +74,8 @@
7575
7676 # Hardcode this for now.
7777 $limit = 48;
78 -
79 - if ( $parval = intval( $par ) ) {
 78+ $parval = intval( $par );
 79+ if ( $parval ) {
8080 if ( $parval <= $limit && $parval > 0 ) {
8181 $limit = $parval;
8282 }
@@ -92,10 +92,12 @@
9393 }
9494
9595 $invertSort = false;
96 - if( $until = $wgRequest->getVal( 'until' ) ) {
 96+ $until = $wgRequest->getVal( 'until' );
 97+ if( $until ) {
9798 $where[] = "img_timestamp < '" . $dbr->timestamp( $until ) . "'";
9899 }
99 - if( $from = $wgRequest->getVal( 'from' ) ) {
 100+ $from = $wgRequest->getVal( 'from' );
 101+ if( $from ) {
100102 $where[] = "img_timestamp >= '" . $dbr->timestamp( $from ) . "'";
101103 $invertSort = true;
102104 }
Index: trunk/phase3/includes/Metadata.php
@@ -119,11 +119,14 @@
120120 protected function person($name, User $user ){
121121 if( $user->isAnon() ){
122122 $this->element( $name, wfMsgExt( 'anonymous', array( 'parsemag' ), 1 ) );
123 - } else if( $real = $user->getRealName() ) {
124 - $this->element( $name, $real );
125123 } else {
126 - $userName = $user->getName();
127 - $this->pageOrString( $name, $user->getUserPage(), wfMsgExt( 'siteuser', 'parsemag', $userName, $userName ) );
 124+ $real = $user->getRealName();
 125+ if( $real ) {
 126+ $this->element( $name, $real );
 127+ } else {
 128+ $userName = $user->getName();
 129+ $this->pageOrString( $name, $user->getUserPage(), wfMsgExt( 'siteuser', 'parsemag', $userName, $userName ) );
 130+ }
128131 }
129132 }
130133
Index: trunk/phase3/includes/Exception.php
@@ -179,7 +179,8 @@
180180 $wgOut->redirect( '' );
181181 $wgOut->clearHTML();
182182
183 - if ( $hookResult = $this->runHooks( get_class( $this ) ) ) {
 183+ $hookResult = $this->runHooks( get_class( $this ) );
 184+ if ( $hookResult ) {
184185 $wgOut->addHTML( $hookResult );
185186 } else {
186187 $wgOut->addHTML( $this->getHTML() );
@@ -187,7 +188,8 @@
188189
189190 $wgOut->output();
190191 } else {
191 - if ( $hookResult = $this->runHooks( get_class( $this ) . "Raw" ) ) {
 192+ $hookResult = $this->runHooks( get_class( $this ) . "Raw" );
 193+ if ( $hookResult ) {
192194 die( $hookResult );
193195 }
194196
Index: trunk/phase3/includes/normal/UtfNormal.php
@@ -308,7 +308,8 @@
309309 $len = $chunk + 1; # Counting down is faster. I'm *so* sorry.
310310
311311 for( $i = -1; --$len; ) {
312 - if( $remaining = $tailBytes[$c = $str{++$i}] ) {
 312+ $remaining = $tailBytes[$c = $str{++$i}]
 313+ if( $remaining ) {
313314 # UTF-8 head byte!
314315 $sequence = $head = $c;
315316 do {

Follow-up revisions

RevisionCommit summaryAuthorDate
r75768Fix minor semi colon fail from r75767reedy00:10, 1 November 2010

Status & tagging log