r56324 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r56323‎ | r56324 | r56325 >
Date:18:58, 14 September 2009
Author:siebrand
Status:resolved (Comments)
Tags:
Comment:
Support GENDER in 'renameuser-renamed-notice', changing LogEventsList::showLogExtract for that:
* param $msgKey from String to Array, where first element is the message key, additional elements are parameters for the key

Comment by Nikerabbit: "... that is cargo train number of parameters :o". I tried not to make it worse.
Modified paths:
  • /trunk/extensions/Renameuser/SpecialRenameuser.i18n.php (modified) (history)
  • /trunk/extensions/Renameuser/SpecialRenameuser.php (modified) (history)
  • /trunk/phase3/includes/LogEventsList.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/LogEventsList.php
@@ -575,10 +575,13 @@
576576 * @param $conds Array Extra conditions for the query
577577 * @param $showIfEmpty boolean Set to false if you don't want any output in case the loglist is empty
578578 * if set to true (default), "No matching items in log" is displayed if loglist is empty
579 - * @param $msgKey String if you want a nice box with a message, set this to the key of the message
 579+ * @param $msgKey Array If you want a nice box with a message, set this
 580+ * to the key of the message. First element is the message
 581+ * key, additional optional elements are parameters for the
 582+ * key that are processed with wgMsgExt and option 'parse'
580583 * @return Integer Number of total log items (not limited by $lim)
581584 */
582 - public static function showLogExtract( &$out, $types=array(), $page='', $user='', $lim=0, $conds=array(), $showIfEmpty = true, $msgKey = '' ) {
 585+ public static function showLogExtract( &$out, $types=array(), $page='', $user='', $lim=0, $conds=array(), $showIfEmpty = true, $msgKey = array() ) {
583586 global $wgUser, $wgOut;
584587 # Insert list of top 50 or so items
585588 $loglist = new LogEventsList( $wgUser->getSkin(), $wgOut, 0 );
@@ -587,8 +590,17 @@
588591 $logBody = $pager->getBody();
589592 $s = '';
590593 if( $logBody ) {
591 - if ( $msgKey )
592 - $s = '<div class="mw-warning-with-logexcerpt">' . wfMsgExt( $msgKey, array('parse') ) ;
 594+ if ( $msgKey ) {
 595+ $s = '<div class="mw-warning-with-logexcerpt">';
 596+
 597+ if ( sizeof( $msgKey ) == 1 ) {
 598+ $s .= wfMsgExt( $msgKey[0], array('parse') );
 599+ } else { // Process additional arguments
 600+ $args = $msgKey;
 601+ unset( $args[0] );
 602+ $s .= wfMsgExt( $msgKey[0], array('parse'), $args );
 603+ }
 604+ }
593605 $s .= $loglist->beginLogEventsList() .
594606 $logBody .
595607 $loglist->endLogEventsList();
Index: trunk/extensions/Renameuser/SpecialRenameuser.i18n.php
@@ -42,7 +42,7 @@
4343 'right-renameuser' => 'Rename users',
4444
4545 'renameuser-renamed-notice' => 'This user has been renamed.
46 -The rename log is provided below for reference.',
 46+The rename log is provided below for reference.', # Supports GENDER
4747 );
4848
4949 /** Message documentation (Message documentation)
@@ -58,6 +58,7 @@
5959 * Parameter $1 is the original username
6060 * Parameter $2 is the new username',
6161 'right-renameuser' => '{{doc-right}}',
 62+ 'renameuser-renamed-notice' => 'This message supports the use of GENDER.',
6263 );
6364
6465 /** Afrikaans (Afrikaans)
Index: trunk/extensions/Renameuser/SpecialRenameuser.php
@@ -47,7 +47,7 @@
4848 if( !$title || $title->getNamespace() !== NS_USER ) {
4949 $rv = ''; // handled in comment, the old way
5050 } else {
51 - $titleLink = $skin ?
 51+ $titleLink = $skin ?
5252 $skin->makeLinkObj( $title, htmlspecialchars( $title->getPrefixedText() ) ) : $title->getText();
5353 # Add title to params
5454 array_unshift( $params, $titleLink );
@@ -70,8 +70,7 @@
7171 if ( $title->getNamespace() == NS_USER || $title->getNamespace() == NS_USER_TALK ) {
7272 // Get the title for the base userpage
7373 $page = Title::makeTitle( NS_USER, str_replace( ' ', '_', $title->getBaseText() ) )->getPrefixedDBkey();
74 - LogEventsList::showLogExtract( $wgOut, 'renameuser', $page, '', 10, array(), false, 'renameuser-renamed-notice' );
 74+ LogEventsList::showLogExtract( $wgOut, 'renameuser', $page, '', 10, array(), false, array( 'renameuser-renamed-notice', $title->getBaseText() ) );
7575 }
7676 return true;
7777 }
78 -

Follow-up revisions

RevisionCommit summaryAuthorDate
r56329Follow-up to r56324 per IRC comments by Nikerabbit: coding style updatedsiebrand19:56, 14 September 2009
r56357* fix oversight in r56324 in showMissingArticle() (string -> array)...siebrand12:41, 15 September 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   00:52, 15 September 2009

Note that this changes the function interface incompatibly; however the new params weren't present in 1.15 and appear to be otherwise unused...

I still think it's kind of icky just on the general phase of parameter bloat though. Reading the call it's very unobvious what the later parameters mean; would be much easier if we pass the more esoteric options in an options array or soemthing.

#Comment by Church of emacs (talk | contribs)   11:39, 15 September 2009

IMHO discussion should take place in r56284, because all those unobvious parameters were added by me in that revision, not in this revision by siebrand. I listed several possible solutions, see Special:Code/MediaWiki/56284#c4042.

#Comment by Siebrand (talk | contribs)   12:43, 15 September 2009

Setting this to fixed for follow-up r56357. Assuming original comment was about r56284.

#Comment by Church of emacs (talk | contribs)   13:31, 15 September 2009

If the meaning of those parameters are a problem, wouldn't an associate array solve the problem (see also my comment on r56284)? Something like
LogEventsList::showLogExtract( array (

'out' => &$out,
'types' => array( 'move', 'delete' ),
'page' => $mypage,
'user' => $myuser,
'lim' => 0,
'conds' => array( 'somecondition' ),
'showIfEmpty' => true,
'msgKey' => 'foobar'

) );

#Comment by Siebrand (talk | contribs)   14:06, 15 September 2009

Yes, that's a case Tim made a while ago on wikitech-l (read the whole thread for a full view, as there are opposing sides).

Status & tagging log