r67707 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r67706‎ | r67707 | r67708 >
Date:11:44, 9 June 2010
Author:demon
Status:resolved (Comments)
Tags:
Comment:
Fixed a bunch of silly instances of [^!=]==\s*(true|false)
Modified paths:
  • /trunk/phase3/config/Installer.php (modified) (history)
  • /trunk/phase3/includes/BagOStuff.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/ImageGallery.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/api/ApiProtect.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseIbm_db2.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)
  • /trunk/phase3/includes/parser/CoreParserFunctions.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOutput.php (modified) (history)
  • /trunk/phase3/languages/LanguageConverter.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/rebuildrecentchanges.php (modified) (history)
  • /trunk/phase3/maintenance/refreshLinks.php (modified) (history)
  • /trunk/phase3/maintenance/runJobs.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/rebuildrecentchanges.php
@@ -216,10 +216,10 @@
217217
218218 $botgroups = $autopatrolgroups = array();
219219 foreach ( $wgGroupPermissions as $group => $rights ) {
220 - if ( isset( $rights['bot'] ) && $rights['bot'] == true ) {
 220+ if ( isset( $rights['bot'] ) && $rights['bot'] ) {
221221 $botgroups[] = $dbw->addQuotes( $group );
222222 }
223 - if ( $wgUseRCPatrol && isset( $rights['autopatrol'] ) && $rights['autopatrol'] == true ) {
 223+ if ( $wgUseRCPatrol && isset( $rights['autopatrol'] ) && $rights['autopatrol'] ) {
224224 $autopatrolgroups[] = $dbw->addQuotes( $group );
225225 }
226226 }
Index: trunk/phase3/maintenance/refreshLinks.php
@@ -176,7 +176,7 @@
177177
178178 $rt = $wgArticle->followRedirect();
179179
180 - if ( $rt == false || !is_object( $rt ) ) {
 180+ if ( !$rt || !is_object( $rt ) ) {
181181 // $wgTitle is not a redirect
182182 // Delete any redirect table entry for it
183183 $dbw->delete( 'redirect', array( 'rd_from' => $id ),
Index: trunk/phase3/maintenance/runJobs.php
@@ -64,11 +64,9 @@
6565 while ( $dbw->selectField( 'job', 'job_id', $conds, 'runJobs.php' ) ) {
6666 $offset = 0;
6767 for ( ; ; ) {
68 - $job = ( $type == false ) ?
69 - Job::pop( $offset )
70 - : Job::pop_type( $type );
 68+ $job = !$type ? Job::pop( $offset ) : Job::pop_type( $type );
7169
72 - if ( $job == false )
 70+ if ( !$job )
7371 break;
7472
7573 wfWaitForSlaves( 5 );
Index: trunk/phase3/includes/BagOStuff.php
@@ -102,7 +102,7 @@
103103 }
104104
105105 public function add( $key, $value, $exptime = 0 ) {
106 - if ( $this->get( $key ) == false ) {
 106+ if ( !$this->get( $key ) ) {
107107 $this->set( $key, $value, $exptime );
108108
109109 return true;
Index: trunk/phase3/includes/GlobalFunctions.php
@@ -1500,7 +1500,7 @@
15011501 pclose( $handle );
15021502 unlink( $mytextName ); unlink( $oldtextName ); unlink( $yourtextName );
15031503
1504 - if ( $result === '' && $old !== '' && $conflict == false ) {
 1504+ if ( $result === '' && $old !== '' && !$conflict ) {
15051505 wfDebug( "Unexpected null result from diff3. Command: $cmd\n" );
15061506 $conflict = true;
15071507 }
Index: trunk/phase3/includes/parser/ParserOutput.php
@@ -28,7 +28,8 @@
2929 $mSections = array(), # Table of contents
3030 $mProperties = array(), # Name/value pairs to be cached in the DB
3131 $mTOCHTML = ''; # HTML of the TOC
32 - private $mIndexPolicy = ''; # 'index' or 'noindex'? Any other value will result in no change.
 32+ private $mIndexPolicy = '', # 'index' or 'noindex'? Any other value will result in no change.
 33+ $mPageIcons = array(); # Array of icons to show for the page (like Protect, Featured, etc)
3334
3435 function ParserOutput( $text = '', $languageLinks = array(), $categoryLinks = array(),
3536 $containsOldMagic = false, $titletext = '' )
@@ -59,6 +60,7 @@
6061 function getWarnings() { return array_keys( $this->mWarnings ); }
6162 function getIndexPolicy() { return $this->mIndexPolicy; }
6263 function getTOCHTML() { return $this->mTOCHTML; }
 64+ function getPageIcons() { return $this->mPageIcons; }
6365
6466 function containsOldMagic() { return $this->mContainsOldMagic; }
6567 function setText( $text ) { return wfSetVar( $this->mText, $text ); }
@@ -251,6 +253,15 @@
252254 }
253255
254256 /**
 257+ * Add page icons to the parser output.
 258+ * @param File $file A valid file
 259+ * @param String $alt Alt text, if any
 260+ */
 261+ function addPageIcon( $file, $alt = '' ) {
 262+ $this->mPageIcons[] = array( $file, $alt );
 263+ }
 264+
 265+ /**
255266 * Override the title to be used for display
256267 * -- this is assumed to have been validated
257268 * (check equal normalisation, etc.)
Index: trunk/phase3/includes/parser/CoreParserFunctions.php
@@ -69,6 +69,7 @@
7070 $parser->setFunctionHook( 'subjectpagenamee', array( __CLASS__, 'subjectpagenamee' ), SFH_NO_HASH );
7171 $parser->setFunctionHook( 'tag', array( __CLASS__, 'tagObj' ), SFH_OBJECT_ARGS );
7272 $parser->setFunctionHook( 'formatdate', array( __CLASS__, 'formatDate' ) );
 73+ $parser->setFunctionHook( 'pageicon', array( __CLASS__, 'pageicon' ) );
7374
7475 if ( $wgAllowDisplayTitle ) {
7576 $parser->setFunctionHook( 'displaytitle', array( __CLASS__, 'displaytitle' ), SFH_NO_HASH );
@@ -628,6 +629,14 @@
629630 }
630631 }
631632
 633+ public static function pageicon( $parser, $name = '', $alt = '' ) {
 634+ $file = wfFindFile( $name );
 635+ if( $file ) {
 636+ $parser->mOutput->addPageIcon( $file, $alt );
 637+ }
 638+ return '';
 639+ }
 640+
632641 /**
633642 * Parser function to extension tag adaptor
634643 */
Index: trunk/phase3/includes/db/DatabaseOracle.php
@@ -249,7 +249,7 @@
250250 $this->mConn = oci_connect( $user, $password, $dbName, $this->defaultCharset, $session_mode );
251251 }
252252
253 - if ( $this->mConn == false ) {
 253+ if ( !$this->mConn ) {
254254 wfDebug( "DB connection error\n" );
255255 wfDebug( "Server: $server, Database: $dbName, User: $user, Password: " . substr( $password, 0, 3 ) . "...\n" );
256256 wfDebug( $this->lastError() . "\n" );
@@ -310,7 +310,7 @@
311311 return false;
312312 }
313313
314 - if ( oci_execute( $stmt, $this->execFlags() ) == false ) {
 314+ if ( !oci_execute( $stmt, $this->execFlags() ) ) {
315315 $e = oci_error( $stmt );
316316 if ( !$this->ignore_DUP_VAL_ON_INDEX || $e['code'] != '1' ) {
317317 $this->reportQueryError( $e['message'], $e['code'], $sql, __FUNCTION__ );
@@ -1013,7 +1013,7 @@
10141014 // Avoid the non-standard "REPLACE INTO" syntax
10151015 echo "<li>Populating interwiki table</li>\n";
10161016 $f = fopen( "../maintenance/interwiki.sql", 'r' );
1017 - if ( $f == false ) {
 1017+ if ( !$f ) {
10181018 dieout( "Could not find the interwiki.sql file" );
10191019 }
10201020
Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -181,7 +181,7 @@
182182 $this->mConn = pg_connect( $connectString );
183183 $phpError = $this->restoreErrorHandler();
184184
185 - if ( $this->mConn == false ) {
 185+ if ( !$this->mConn ) {
186186 wfDebug( "DB connection error\n" );
187187 wfDebug( "Server: $server, Database: $dbName, User: $user, Password: " . substr( $password, 0, 3 ) . "...\n" );
188188 wfDebug( $this->lastError()."\n" );
@@ -309,7 +309,7 @@
310310 $connectVars['password'] = $password;
311311
312312 @$this->mConn = pg_connect( $this->makeConnectionString( $connectVars ) );
313 - if ( $this->mConn == false ) {
 313+ if ( $this->mConn ) {
314314 print "<b>FAILED TO CONNECT!</b></li>";
315315 dieout("</ul>");
316316 }
@@ -1355,7 +1355,7 @@
13561356 echo "<li>Populating interwiki table... ";
13571357 ## Avoid the non-standard "REPLACE INTO" syntax
13581358 $f = fopen( "../maintenance/interwiki.sql", 'r' );
1359 - if ($f == false ) {
 1359+ if ( $f ) {
13601360 print "<b>FAILED</b></li>";
13611361 dieout( "Could not find the interwiki.sql file" );
13621362 }
Index: trunk/phase3/includes/db/DatabaseIbm_db2.php
@@ -520,7 +520,7 @@
521521 // Rather, turn autocommit off in the begin function and turn on after a commit
522522 db2_autocommit($this->mConn, DB2_AUTOCOMMIT_ON);
523523
524 - if ( $this->mConn == false ) {
 524+ if ( !$this->mConn ) {
525525 $this->installPrint( "DB connection error\n" );
526526 $this->installPrint( "Server: $server, Database: $dbName, User: $user, Password: " . substr( $password, 0, 3 ) . "...\n" );
527527 $this->installPrint( $this->lastError()."\n" );
Index: trunk/phase3/includes/db/DatabaseSqlite.php
@@ -494,7 +494,7 @@
495495
496496 # Use DatabasePostgres's code to populate interwiki from MySQL template
497497 $f = fopen( "$IP/maintenance/interwiki.sql", 'r' );
498 - if ( $f == false ) {
 498+ if ( !$f ) {
499499 dieout( "Could not find the interwiki.sql file." );
500500 }
501501
Index: trunk/phase3/includes/EditPage.php
@@ -2628,7 +2628,7 @@
26292629 }
26302630
26312631 function getBaseRevision() {
2632 - if ( $this->mBaseRevision == false ) {
 2632+ if ( !$this->mBaseRevision ) {
26332633 $db = wfGetDB( DB_MASTER );
26342634 $baseRevision = Revision::loadFromTimestamp(
26352635 $db, $this->mTitle, $this->edittime );
Index: trunk/phase3/includes/OutputPage.php
@@ -39,6 +39,7 @@
4040 var $mParseWarnings = array();
4141 var $mSquidMaxage = 0;
4242 var $mRevisionId = null;
 43+ var $mPageIcons = array();
4344 protected $mTitle = null;
4445
4546 /**
@@ -1079,6 +1080,7 @@
10801081 $this->addCategoryLinks( $parserOutput->getCategories() );
10811082 $this->mNewSectionLink = $parserOutput->getNewSection();
10821083 $this->mHideNewSectionLink = $parserOutput->getHideNewSection();
 1084+ $this->mPageIcons = $parserOutput->getPageIcons();
10831085
10841086 $this->mParseWarnings = $parserOutput->getWarnings();
10851087 if ( !$parserOutput->isCacheable() ) {
Index: trunk/phase3/includes/api/ApiProtect.php
@@ -95,7 +95,7 @@
9696 $expiryarray[$p[0]] = Block::infinity();
9797 } else {
9898 $exp = strtotime( $expiry[$i] );
99 - if ( $exp < 0 || $exp == false ) {
 99+ if ( $exp < 0 || !$exp ) {
100100 $this->dieUsageMsg( array( 'invalidexpiry', $expiry[$i] ) );
101101 }
102102
Index: trunk/phase3/includes/Title.php
@@ -824,7 +824,7 @@
825825 }
826826
827827 // internal links should point to same variant as current page (only anonymous users)
828 - if ( $variant == false && $wgContLang->hasVariants() && !$wgUser->isLoggedIn() ) {
 828+ if ( !$variant && $wgContLang->hasVariants() && !$wgUser->isLoggedIn() ) {
829829 $pref = $wgContLang->getPreferredVariant( false );
830830 if ( $pref != $wgContLang->getCode() )
831831 $variant = $pref;
@@ -843,7 +843,7 @@
844844 $dbkey = wfUrlencode( $this->getPrefixedDBkey() );
845845 if ( $query == '' ) {
846846 if ( $variant != false && $wgContLang->hasVariants() ) {
847 - if ( $wgVariantArticlePath == false ) {
 847+ if ( !$wgVariantArticlePath ) {
848848 $variantArticlePath = "$wgScript?title=$1&variant=$2"; // default
849849 } else {
850850 $variantArticlePath = $wgVariantArticlePath;
@@ -1474,7 +1474,7 @@
14751475 $scBlockExpiryOptions = wfMsg( 'ipboptions' );
14761476
14771477 foreach ( explode( ',', $scBlockExpiryOptions ) as $option ) {
1478 - if ( strpos( $option, ':' ) == false )
 1478+ if ( !strpos( $option, ':' ) )
14791479 continue;
14801480
14811481 list ( $show, $value ) = explode( ":", $option );
Index: trunk/phase3/includes/SkinTemplate.php
@@ -230,6 +230,21 @@
231231 $tpl->set( 'pageclass', $this->getPageClasses( $this->mTitle ) );
232232 $tpl->set( 'skinnameclass', ( 'skin-' . Sanitizer::escapeClass( $this->getSkinName() ) ) );
233233
 234+ $icons = '';
 235+ foreach( $out->mPageIcons as $icon ) {
 236+ list( $file, $alt ) = $icon;
 237+ $fileAttr = array( 'src' => $file->createThumb( 16 ) );
 238+ if( $alt != '' ) {
 239+ $msg = wfMsg( $alt );
 240+ if( !wfEmptyMsg( $alt ) ) {
 241+ $alt = $msg;
 242+ }
 243+ $fileAttr['alt'] = htmlspecialchars( $alt );
 244+ }
 245+ $icons .= Html::element( 'img', $fileAttr ) . "&nbsp;";
 246+ }
 247+ $tpl->set( 'pageicons', $icons );
 248+
234249 $nsname = MWNamespace::exists( $this->mTitle->getNamespace() ) ?
235250 MWNamespace::getCanonicalName( $this->mTitle->getNamespace() ) :
236251 $this->mTitle->getNsText();
Index: trunk/phase3/includes/SpecialPage.php
@@ -698,7 +698,7 @@
699699 $this->mRestriction = $restriction;
700700 $this->mListed = $listed;
701701 $this->mIncludable = $includable;
702 - if ( $function == false ) {
 702+ if ( !$function ) {
703703 $this->mFunction = 'wfSpecial'.$name;
704704 } else {
705705 $this->mFunction = $function;
Index: trunk/phase3/includes/ImageGallery.php
@@ -183,7 +183,7 @@
184184 * @param $f Boolean: set to false to disable.
185185 */
186186 function setShowBytes( $f ) {
187 - $this->mShowBytes = ( $f == true);
 187+ $this->mShowBytes = $f;
188188 }
189189
190190 /**
@@ -193,7 +193,7 @@
194194 * @param $f Boolean: set to false to disable.
195195 */
196196 function setShowFilename( $f ) {
197 - $this->mShowFilename = ( $f == true);
 197+ $this->mShowFilename = $f;
198198 }
199199
200200 /**
Index: trunk/phase3/config/Installer.php
@@ -1330,7 +1330,7 @@
13311331 $localSettings = str_replace( "\r\n", "\n", $localSettings );
13321332 $f = fopen( "LocalSettings.php", 'xt' );
13331333
1334 - if( $f == false ) {
 1334+ if( !$f ) {
13351335 print( "</li>\n" );
13361336 dieout( "<p>Couldn't write out LocalSettings.php. Check that the directory permissions are correct and that there isn't already a file of that name here...</p>\n" .
13371337 "<p>Here's the file that would have been written, try to paste it into place manually:</p>\n" .
Index: trunk/phase3/languages/LanguageConverter.php
@@ -698,7 +698,7 @@
699699 }
700700
701701 $variants = $this->autoConvertToAllVariants( $link );
702 - if ( $variants == false ) { // give up
 702+ if ( !$variants ) { // give up
703703 return;
704704 }
705705
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -356,6 +356,7 @@
357357 'url_path' => array( 0, 'PATH' ),
358358 'url_wiki' => array( 0, 'WIKI' ),
359359 'url_query' => array( 0, 'QUERY' ),
 360+ 'pageicon' => array( 1, 'pageicon' ),
360361 );
361362
362363 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r67708Partial revert r67707, included half-baked patchdemon11:54, 9 June 2010
r67727Followup r67707, cast these to boolean just to be paranoiddemon15:06, 9 June 2010
r68716Fixed a fatal installer failure on PG introduced in r67707. 1.16 is unaffectedmaxsem09:11, 29 June 2010
r68717Fixed yet another borkage from r67707maxsem09:20, 29 June 2010

Comments

#Comment by Platonides (talk | contribs)   14:56, 9 June 2010
  1. I think some instances are better with an explicit false.
  2. The changes in ImageGallery could lead to a different result if it is not passed a boolean. You may want to cast it.
#Comment by 😂 (talk | contribs)   15:07, 9 June 2010

I resolved #2. Which ones, specifically in #1?

Status & tagging log