r44031 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r44030‎ \| r44031 \| r44032 >
Date:	23:06, 28 November 2008
Author:	aaron
Status:	ok
Tags:
Comment:	Various minor code style tweaks and cleanup to RC
Modified paths:	/trunk/phase3/includes/ChangesList.php (modified) (history) /trunk/phase3/includes/specials/SpecialRecentchanges.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ChangesList.php
—	—	@@ -3,10 +3,9 @@
4	4	/**
5	5	* @todo document
6	6	*/
7		~~-class RCCacheEntry extends RecentChange~~
8		-{
	7	+class RCCacheEntry extends RecentChange {
9	8	var $secureName, $link;
10		~~- var $curlink , $difflink, $lastlink , $usertalklink , $versionlink ;~~
	9	+ var $curlink , $difflink, $lastlink, $usertalklink, $versionlink;
11	10	var $userlink, $timestamp, $watched;
12	11
13	12	static function newFromParent( $rc ) {
—	—	@@ -15,7 +14,7 @@
16	15	$rc2->mExtra = $rc->mExtra;
17	16	return $rc2;
18	17	}
19		~~-} ;~~
	18	+}
20	19
21	20	/**
22	21	* Class to show various lists of changes:
—	—	@@ -25,13 +24,12 @@
26	25	*/
27	26	class ChangesList {
28	27	# Called by history lists and recent changes
29		~~- #~~
30	28
31	29	/**
32	30	* Changeslist contructor
33	31	* @param Skin $skin
34	32	*/
35		~~- function __construct( &$skin ) {~~
	33	+ public function __construct( &$skin ) {
36	34	$this->skin =& $skin;
37	35	$this->preCacheMessages();
38	36	}
—	—	@@ -58,7 +56,6 @@
59	57	* they are called often, we call them once and save them in $this->message
60	58	*/
61	59	private function preCacheMessages() {
62		~~- // Precache various messages~~
63	60	if( !isset( $this->message ) ) {
64	61	foreach( explode(' ', 'cur diff hist minoreditletter newpageletter last '.
65	62	'blocklink history boteditletter semicolon-separator' ) as $msg ) {
—	—	@@ -78,10 +75,10 @@
79	76	* @return string
80	77	*/
81	78	protected function recentChangesFlags( $new, $minor, $patrolled, $nothing = ' ', $bot = false ) {
82		~~- $f = $new ? '<span class="newpage">' . $this->message['newpageletter'] . '</span>'~~
83		~~- : $nothing;~~
84		~~- $f .= $minor ? '<span class="minor">' . $this->message['minoreditletter'] . '</span>'~~
85		~~- : $nothing;~~
	79	+ $f = $new ?
	80	+ '<span class="newpage">' . $this->message['newpageletter'] . '</span>' : $nothing;
	81	+ $f .= $minor ?
	82	+ '<span class="minor">' . $this->message['minoreditletter'] . '</span>' : $nothing;
86	83	$f .= $bot ? '<span class="bot">' . $this->message['boteditletter'] . '</span>' : $nothing;
87	84	$f .= $patrolled ? '<span class="unpatrolled">!</span>' : $nothing;
88	85	return $f;
—	—	@@ -142,14 +139,13 @@
143	140	# Hist
144	141	$s .= $this->skin->makeKnownLinkObj( $rc->getMovedToTitle(), $this->message['hist'], 'action=history' ) .
145	142	') . . ';
146		-
147	143	# "[[x]] moved to [[y]]"
148	144	$msg = ( $rc->mAttribs['rc_type'] == RC_MOVE ) ? '1movedto2' : '1movedto2_redir';
149	145	$s .= wfMsg( $msg, $this->skin->makeKnownLinkObj( $rc->getTitle(), '', 'redirect=no' ),
150	146	$this->skin->makeKnownLinkObj( $rc->getMovedToTitle(), '' ) );
151	147	}
152	148
153		~~- protected function insertDateHeader(&$s, $rc_timestamp) {~~
	149	+ protected function insertDateHeader( &$s, $rc_timestamp ) {
154	150	global $wgLang;
155	151
156	152	# Make date header if necessary
—	—	@@ -164,21 +160,20 @@
165	161	}
166	162	}
167	163
168		~~- protected function insertLog(&$s, $title, $logtype) {~~
	164	+ protected function insertLog( &$s, $title, $logtype ) {
169	165	$logname = LogPage::logName( $logtype );
170	166	$s .= '(' . $this->skin->makeKnownLinkObj($title, $logname ) . ')';
171	167	}
172	168
173		~~- protected function insertDiffHist(&$s, &$rc, $unpatrolled) {~~
	169	+ protected function insertDiffHist( &$s, &$rc, $unpatrolled ) {
174	170	# Diff link
175	171	if( !$this->userCan($rc,Revision::DELETED_TEXT) ) {
176	172	$diffLink = $this->message['diff'];
177	173	} else if( $rc->mAttribs['rc_type'] == RC_NEW \|\| $rc->mAttribs['rc_type'] == RC_LOG ) {
178	174	$diffLink = $this->message['diff'];
179	175	} else {
180		~~- $rcidparam = $unpatrolled~~
181		~~- ? array( 'rcid' => $rc->mAttribs['rc_id'] )~~
182		~~- : array();~~
	176	+ $rcidparam = $unpatrolled ?
	177	+ array( 'rcid' => $rc->mAttribs['rc_id'] ) : array();
183	178	$diffLink = $this->skin->makeKnownLinkObj( $rc->getTitle(), $this->message['diff'],
184	179	wfArrayToCGI( array(
185	180	'curid' => $rc->mAttribs['rc_cur_id'],
—	—	@@ -188,7 +183,6 @@
189	184	'', '', ' tabindex="'.$rc->counter.'"');
190	185	}
191	186	$s .= '('.$diffLink.') (';
192		-
193	187	# History link
194	188	$s .= $this->skin->makeKnownLinkObj( $rc->getTitle(), $this->message['hist'],
195	189	wfArrayToCGI( array(
—	—	@@ -197,31 +191,32 @@
198	192	$s .= ') . . ';
199	193	}
200	194
201		~~- protected function insertArticleLink(&$s, &$rc, $unpatrolled, $watched) {~~
202		~~- # Article link~~
	195	+ protected function insertArticleLink( &$s, &$rc, $unpatrolled, $watched ) {
	196	+ global $wgContLang;
203	197	# If it's a new article, there is no diff link, but if it hasn't been
204	198	# patrolled yet, we need to give users a way to do so
205		~~- $params = ( $unpatrolled && $rc->mAttribs['rc_type'] == RC_NEW )~~
206		~~- ? 'rcid='.$rc->mAttribs['rc_id']~~
207		~~- : '';~~
	199	+ $params = ( $unpatrolled && $rc->mAttribs['rc_type'] == RC_NEW ) ?
	200	+ 'rcid='.$rc->mAttribs['rc_id'] : '';
208	201	if( $this->isDeleted($rc,Revision::DELETED_TEXT) ) {
209	202	$articlelink = $this->skin->makeKnownLinkObj( $rc->getTitle(), '', $params );
210	203	$articlelink = '<span class="history-deleted">'.$articlelink.'</span>';
211	204	} else {
212	205	$articlelink = ' '. $this->skin->makeKnownLinkObj( $rc->getTitle(), '', $params );
213	206	}
214		~~- if( $watched )~~
	207	+ # Bolden pages watched by this user
	208	+ if( $watched ) {
215	209	$articlelink = "<strong class=\"mw-watched\">{$articlelink}</strong>";
216		~~- global $wgContLang;~~
	210	+ }
	211	+ # RTL/LTR marker
217	212	$articlelink .= $wgContLang->getDirMark();
218	213
219	214	wfRunHooks('ChangesListInsertArticleLink',
220	215	array(&$this, &$articlelink, &$s, &$rc, $unpatrolled, $watched));
221	216
222		~~- $s .= ' '.$articlelink;~~
	217	+ $s .= " $articlelink";
223	218	}
224	219
225		~~- protected function insertTimestamp(&$s, $rc) {~~
	220	+ protected function insertTimestamp( &$s, $rc ) {
226	221	global $wgLang;
227	222	# Timestamp
228	223	$s .= $this->message['semicolon-separator'] . $wgLang->time( $rc->mAttribs['rc_timestamp'], true, true ) . ' . . ';
Index: trunk/phase3/includes/specials/SpecialRecentchanges.php
—	—	@@ -50,7 +50,7 @@
51	51	$opts->fetchValuesFromRequest( $wgRequest );
52	52
53	53	// Give precedence to subpage syntax
54		~~- if ( $parameters !== null ) {~~
	54	+ if( $parameters !== null ) {
55	55	$this->parseParameters( $parameters, $opts );
56	56	}
57	57
—	—	@@ -105,14 +105,14 @@
106	106	}
107	107
108	108	foreach( $rows as $row ) {
109		~~- if ( !$feedFormat ) {~~
	109	+ if( !$feedFormat ) {
110	110	// User page and talk links
111	111	$batch->add( NS_USER, $row->rc_user_text );
112	112	$batch->add( NS_USER_TALK, $row->rc_user_text );
113	113	}
114	114	}
115	115
116		~~- if ( $feedFormat ) {~~
	116	+ if( $feedFormat ) {
117	117	list( $feed, $feedObj ) = $this->getFeedObject( $feedFormat );
118	118	$feed->execute( $feedObj, $rows, $opts['limit'], $opts['hideminor'], $lastmod );
119	119	} else {
—	—	@@ -146,21 +146,21 @@
147	147	*/
148	148	public function parseParameters( $par, FormOptions $opts ) {
149	149	$bits = preg_split( '/\s,\s/', trim( $par ) );
150		~~- foreach ( $bits as $bit ) {~~
151		~~- if ( 'hidebots' === $bit ) $opts['hidebots'] = true;~~
152		~~- if ( 'bots' === $bit ) $opts['hidebots'] = false;~~
153		~~- if ( 'hideminor' === $bit ) $opts['hideminor'] = true;~~
154		~~- if ( 'minor' === $bit ) $opts['hideminor'] = false;~~
155		~~- if ( 'hideliu' === $bit ) $opts['hideliu'] = true;~~
156		~~- if ( 'hidepatrolled' === $bit ) $opts['hidepatrolled'] = true;~~
157		~~- if ( 'hideanons' === $bit ) $opts['hideanons'] = true;~~
158		~~- if ( 'hidemyself' === $bit ) $opts['hidemyself'] = true;~~
	150	+ foreach( $bits as $bit ) {
	151	+ if( 'hidebots' === $bit ) $opts['hidebots'] = true;
	152	+ if( 'bots' === $bit ) $opts['hidebots'] = false;
	153	+ if( 'hideminor' === $bit ) $opts['hideminor'] = true;
	154	+ if( 'minor' === $bit ) $opts['hideminor'] = false;
	155	+ if( 'hideliu' === $bit ) $opts['hideliu'] = true;
	156	+ if( 'hidepatrolled' === $bit ) $opts['hidepatrolled'] = true;
	157	+ if( 'hideanons' === $bit ) $opts['hideanons'] = true;
	158	+ if( 'hidemyself' === $bit ) $opts['hidemyself'] = true;
159	159
160		~~- if ( is_numeric( $bit ) ) $opts['limit'] = $bit;~~
	160	+ if( is_numeric( $bit ) ) $opts['limit'] = $bit;
161	161
162	162	$m = array();
163		~~- if ( preg_match( '/^limit=(\d+)$/', $bit, $m ) ) $opts['limit'] = $m[1];~~
164		~~- if ( preg_match( '/^days=(\d+)$/', $bit, $m ) ) $opts['days'] = $m[1];~~
	163	+ if( preg_match( '/^limit=(\d+)$/', $bit, $m ) ) $opts['limit'] = $m[1];
	164	+ if( preg_match( '/^days=(\d+)$/', $bit, $m ) ) $opts['days'] = $m[1];
165	165	}
166	166	}
167	167
—	—	@@ -176,7 +176,7 @@
177	177	global $wgUseRCPatrol, $wgOut;
178	178	$dbr = wfGetDB( DB_SLAVE );
179	179	$lastmod = $dbr->selectField( 'recentchanges', 'MAX(rc_timestamp)', false, __FUNCTION__ );
180		~~- if ( $feedFormat \|\| !$wgUseRCPatrol ) {~~
	180	+ if( $feedFormat \|\| !$wgUseRCPatrol ) {
181	181	if( $lastmod && $wgOut->checkLastModified( $lastmod ) ){
182	182	# Client cache fresh and headers sent, nothing more to do.
183	183	return false;
—	—	@@ -229,12 +229,12 @@
230	230	$hideLoggedInUsers = $opts['hideliu'] && !$forcebot;
231	231	$hideAnonymousUsers = $opts['hideanons'] && !$forcebot;
232	232
233		~~- if ( $opts['hideminor'] ) $conds['rc_minor'] = 0;~~
234		~~- if ( $opts['hidebots'] ) $conds['rc_bot'] = 0;~~
235		~~- if ( $hidePatrol ) $conds['rc_patrolled'] = 0;~~
236		~~- if ( $forcebot ) $conds['rc_bot'] = 1;~~
237		~~- if ( $hideLoggedInUsers ) $conds[] = 'rc_user = 0';~~
238		~~- if ( $hideAnonymousUsers ) $conds[] = 'rc_user != 0';~~
	233	+ if( $opts['hideminor'] ) $conds['rc_minor'] = 0;
	234	+ if( $opts['hidebots'] ) $conds['rc_bot'] = 0;
	235	+ if( $hidePatrol ) $conds['rc_patrolled'] = 0;
	236	+ if( $forcebot ) $conds['rc_bot'] = 1;
	237	+ if( $hideLoggedInUsers ) $conds[] = 'rc_user = 0';
	238	+ if( $hideAnonymousUsers ) $conds[] = 'rc_user != 0';
239	239
240	240	if( $opts['hidemyself'] ) {
241	241	if( $wgUser->getId() ) {
—	—	@@ -245,8 +245,8 @@
246	246	}
247	247
248	248	# Namespace filtering
249		~~- if ( $opts['namespace'] !== '' ) {~~
250		~~- if ( !$opts['invert'] ) {~~
	249	+ if( $opts['namespace'] !== '' ) {
	250	+ if( !$opts['invert'] ) {
251	251	$conds[] = 'rc_namespace = ' . $dbr->addQuotes( $opts['namespace'] );
252	252	} else {
253	253	$conds[] = 'rc_namespace != ' . $dbr->addQuotes( $opts['namespace'] );
—	—	@@ -278,7 +278,8 @@
279	279	// JOIN on watchlist for users
280	280	if( $uid ) {
281	281	$tables[] = 'watchlist';
282		~~- $join_conds = array( 'watchlist' => array('LEFT JOIN',"wl_user={$uid} AND wl_title=rc_title AND wl_namespace=rc_namespace") );~~
	282	+ $join_conds = array( 'watchlist' => array('LEFT JOIN',
	283	+ "wl_user={$uid} AND wl_title=rc_title AND wl_namespace=rc_namespace") );
283	284	}
284	285
285	286	wfRunHooks('SpecialRecentChangesQuery', array( &$conds, &$tables, &$join_conds, $opts ) );
—	—	@@ -326,7 +327,7 @@
327	328
328	329	$limit = $opts['limit'];
329	330
330		~~- if ( !$this->including() ) {~~
	331	+ if( !$this->including() ) {
331	332	// Output options box
332	333	$this->doHeader( $opts );
333	334	}
—	—	@@ -334,55 +335,47 @@
335	336	// And now for the content
336	337	$wgOut->setSyndicated( true );
337	338
338		~~- $list = ChangesList::newFromUser( $wgUser );~~
339		-
340		~~- if ( $wgAllowCategorizedRecentChanges ) {~~
	339	+ if( $wgAllowCategorizedRecentChanges ) {
341	340	$this->filterByCategories( $rows, $opts );
342	341	}
343	342
344		~~- $s = $list->beginRecentChangesList();~~
345		~~- $counter = 1;~~
346		-
347	343	$showWatcherCount = $wgRCShowWatchingUsers && $wgUser->getOption( 'shownumberswatching' );
348	344	$watcherCache = array();
349	345
350	346	$dbr = wfGetDB( DB_SLAVE );
351	347
352		~~- foreach( $rows as $obj ){~~
353		~~- if( $limit == 0) {~~
	348	+ $counter = 1;
	349	+ $list = ChangesList::newFromUser( $wgUser );
	350	+
	351	+ $s = $list->beginRecentChangesList();
	352	+ foreach( $rows as $obj ) {
	353	+ if( $limit == 0 ) {
354	354	break;
355	355	}
	356	+ $rc = RecentChange::newFromRow( $obj );
	357	+ $rc->counter = $counter++;
356	358
357		~~- if ( ! ( $opts['hideminor'] && $obj->rc_minor ) &&~~
358		~~- ! ( $opts['hidepatrolled'] && $obj->rc_patrolled ) ) {~~
359		~~- $rc = RecentChange::newFromRow( $obj );~~
360		~~- $rc->counter = $counter++;~~
	359	+ $rc->notificationtimestamp = false; // Default
	360	+ if( $wgShowUpdatedMarker && !empty($obj->wl_notificationtimestamp) ) {
	361	+ $rc->notificationtimestamp = ($obj->rc_timestamp >= $obj->wl_notificationtimestamp);
	362	+ }
361	363
362		~~- if ($wgShowUpdatedMarker~~
363		~~- && !empty( $obj->wl_notificationtimestamp )~~
364		~~- && ($obj->rc_timestamp >= $obj->wl_notificationtimestamp)) {~~
365		~~- $rc->notificationtimestamp = true;~~
366		~~- } else {~~
367		~~- $rc->notificationtimestamp = false;~~
	364	+ $rc->numberofWatchingusers = 0; // Default
	365	+ if( $showWatcherCount && $obj->rc_namespace >= 0 ) {
	366	+ if( !isset($watcherCache[$obj->rc_namespace][$obj->rc_title]) ) {
	367	+ $watcherCache[$obj->rc_namespace][$obj->rc_title] =
	368	+ $dbr->selectField( 'watchlist',
	369	+ 'COUNT(*)',
	370	+ array(
	371	+ 'wl_namespace' => $obj->rc_namespace,
	372	+ 'wl_title' => $obj->rc_title,
	373	+ ),
	374	+ __METHOD__ . '-watchers' );
368	375	}
369		-
370		~~- $rc->numberofWatchingusers = 0; // Default~~
371		~~- if ($showWatcherCount && $obj->rc_namespace >= 0) {~~
372		~~- if (!isset($watcherCache[$obj->rc_namespace][$obj->rc_title])) {~~
373		~~- $watcherCache[$obj->rc_namespace][$obj->rc_title] =~~
374		~~- $dbr->selectField( 'watchlist',~~
375		~~- 'COUNT(*)',~~
376		~~- array(~~
377		~~- 'wl_namespace' => $obj->rc_namespace,~~
378		~~- 'wl_title' => $obj->rc_title,~~
379		~~- ),~~
380		~~- __METHOD__ . '-watchers' );~~
381		~~- }~~
382		~~- $rc->numberofWatchingusers = $watcherCache[$obj->rc_namespace][$obj->rc_title];~~
383		~~- }~~
384		~~- $s .= $list->recentChangesLine( $rc, !empty( $obj->wl_user ) );~~
385		~~- --$limit;~~
	376	+ $rc->numberofWatchingusers = $watcherCache[$obj->rc_namespace][$obj->rc_title];
386	377	}
	378	+ $s .= $list->recentChangesLine( $rc, !empty( $obj->wl_user ) );
	379	+ --$limit;
387	380	}
388	381	$s .= $list->endRecentChangesList();
389	382	$wgOut->addHTML( $s );
—	—	@@ -413,13 +406,13 @@
414	407	$submit = ' ' . Xml::submitbutton( wfMsg( 'allpagessubmit' ) );
415	408
416	409	$out = Xml::openElement( 'table', array( 'class' => 'mw-recentchanges-table' ) );
417		~~- foreach ( $extraOpts as $optionRow ) {~~
	410	+ foreach( $extraOpts as $optionRow ) {
418	411	# Add submit button to the last row only
419	412	++$count;
420	413	$addSubmit = $count === $extraOptsCount ? $submit : '';
421	414
422	415	$out .= Xml::openElement( 'tr' );
423		~~- if ( is_array( $optionRow ) ) {~~
	416	+ if( is_array( $optionRow ) ) {
424	417	$out .= Xml::tags( 'td', array( 'class' => 'mw-label' ), $optionRow[0] );
425	418	$out .= Xml::tags( 'td', array( 'class' => 'mw-input' ), $optionRow[1] . $addSubmit );
426	419	} else {
—	—	@@ -430,7 +423,7 @@
431	424	$out .= Xml::closeElement( 'table' );
432	425
433	426	$unconsumed = $opts->getUnconsumedValues();
434		~~- foreach ( $unconsumed as $key => $value ) {~~
	427	+ foreach( $unconsumed as $key => $value ) {
435	428	$out .= Xml::hidden( $key, $value );
436	429	}
437	430
—	—	@@ -458,7 +451,7 @@
459	452	$extraOpts['namespace'] = $this->namespaceFilterForm( $opts );
460	453
461	454	global $wgAllowCategorizedRecentChanges;
462		~~- if ( $wgAllowCategorizedRecentChanges ) {~~
	455	+ if( $wgAllowCategorizedRecentChanges ) {
463	456	$extraOpts['category'] = $this->categoryFilterForm( $opts );
464	457	}
465	458
—	—	@@ -529,30 +522,30 @@
530	523
531	524	# Filter categories
532	525	$cats = array();
533		~~- foreach ( $categories as $cat ) {~~
	526	+ foreach( $categories as $cat ) {
534	527	$cat = trim( $cat );
535		~~- if ( $cat == "" ) continue;~~
	528	+ if( $cat == "" ) continue;
536	529	$cats[] = $cat;
537	530	}
538	531
539	532	# Filter articles
540	533	$articles = array();
541	534	$a2r = array();
542		~~- foreach ( $rows AS $k => $r ) {~~
	535	+ foreach( $rows AS $k => $r ) {
543	536	$nt = Title::makeTitle( $r->rc_namespace, $r->rc_title );
544	537	$id = $nt->getArticleID();
545		~~- if ( $id == 0 ) continue; # Page might have been deleted...~~
546		~~- if ( !in_array($id, $articles) ) {~~
	538	+ if( $id == 0 ) continue; # Page might have been deleted...
	539	+ if( !in_array($id, $articles) ) {
547	540	$articles[] = $id;
548	541	}
549		~~- if ( !isset($a2r[$id]) ) {~~
	542	+ if( !isset($a2r[$id]) ) {
550	543	$a2r[$id] = array();
551	544	}
552	545	$a2r[$id][] = $k;
553	546	}
554	547
555	548	# Shortcut?
556		~~- if ( !count($articles) \|\| !count($cats) )~~
	549	+ if( !count($articles) \|\| !count($cats) )
557	550	return ;
558	551
559	552	# Look up
—	—	@@ -562,8 +555,8 @@
563	556
564	557	# Filter
565	558	$newrows = array();
566		~~- foreach ( $match AS $id ) {~~
567		~~- foreach ( $a2r[$id] AS $rev ) {~~
	559	+ foreach( $match AS $id ) {
	560	+ foreach( $a2r[$id] AS $rev ) {
568	561	$k = $rev;
569	562	$newrows[$k] = $rows[$k];
570	563	}

Status & tagging log

18:37, 10 December 2008 Brion VIBBER (talk | contribs) changed the status of r44031 [removed: new added: ok]