r51200 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r51199‎ | r51200 | r51201 >
Date:17:27, 30 May 2009
Author:soxred93
Status:reverted (Comments)
Tags:
Comment:
(bug 19012) Added {{NUMBEROFCONTRIBS}} variable
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/MagicWord.php (modified) (history)
  • /trunk/phase3/includes/parser/CoreParserFunctions.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/CoreParserFunctions.php
——@@ -38,6 +38,7 @@
3939 $parser->setFunctionHook( 'numberingroup', array( __CLASS__, 'numberingroup' ), SFH_NO_HASH );
4040 $parser->setFunctionHook( 'numberofedits', array( __CLASS__, 'numberofedits' ), SFH_NO_HASH );
4141 $parser->setFunctionHook( 'numberofviews', array( __CLASS__, 'numberofviews' ), SFH_NO_HASH );
 42+ $parser->setFunctionHook( 'numberofcontribs', array( __CLASS__, 'numberofcontribs' ), SFH_NO_HASH );
4243 $parser->setFunctionHook( 'language', array( __CLASS__, 'language' ), SFH_NO_HASH );
4344 $parser->setFunctionHook( 'padleft', array( __CLASS__, 'padleft' ), SFH_NO_HASH );
4445 $parser->setFunctionHook( 'padright', array( __CLASS__, 'padright' ), SFH_NO_HASH );
——@@ -571,7 +572,7 @@
572573 $rev = Revision::newFromTitle($title);
573574 $id = $rev ? $rev->getPage() : 0;
574575 $length = $cache[$page] = $rev ? $rev->getSize() : 0;
575 -
 576+
576577 // Register dependency in templatelinks
577578 $parser->mOutput->addTemplate( $title, $id, $rev ? $rev->getId() : 0 );
578579 }
——@@ -593,7 +594,20 @@
594595 $lang = $wgContLang->getLanguageName( strtolower( $arg ) );
595596 return $lang != '' ? $lang : $arg;
596597 }
 598+
 599+ /**
 600+ * Returns the number of contributions by a certain user
 601+ */
 602+ static function numberofcontribs( $parser, $user = null ) {
 603+ if ( is_null($user) || !User::isValidUserName( $user ) )
 604+ return '';
 605+
 606+ $u = User::newFromName( $user );
 607+ $u->load();
597608
 609+ return wfEscapeWikiText( $u->edits( $u->mId ) );
 610+ }
 611+
598612 /**
599613 * Unicode-safe str_pad with the restriction that $length is forced to be <= 500
600614 */
Index: trunk/phase3/includes/MagicWord.php
——@@ -107,45 +107,46 @@
108108
109109 /* Array of caching hints for ParserCache */
110110 static public $mCacheTTLs = array (
111 - 'currentmonth' => 86400,
112 - 'currentmonth1' => 86400,
113 - 'currentmonthname' => 86400,
114 - 'currentmonthnamegen' => 86400,
115 - 'currentmonthabbrev' => 86400,
116 - 'currentday' => 3600,
117 - 'currentday2' => 3600,
118 - 'currentdayname' => 3600,
119 - 'currentyear' => 86400,
120 - 'currenttime' => 3600,
121 - 'currenthour' => 3600,
122 - 'localmonth' => 86400,
123 - 'localmonth1' => 86400,
124 - 'localmonthname' => 86400,
125 - 'localmonthnamegen' => 86400,
126 - 'localmonthabbrev' => 86400,
127 - 'localday' => 3600,
128 - 'localday2' => 3600,
129 - 'localdayname' => 3600,
130 - 'localyear' => 86400,
131 - 'localtime' => 3600,
132 - 'localhour' => 3600,
133 - 'numberofarticles' => 3600,
134 - 'numberoffiles' => 3600,
135 - 'numberofedits' => 3600,
136 - 'currentweek' => 3600,
137 - 'currentdow' => 3600,
138 - 'localweek' => 3600,
139 - 'localdow' => 3600,
140 - 'numberofusers' => 3600,
141 - 'numberofactiveusers' => 3600,
142 - 'numberofpages' => 3600,
143 - 'currentversion' => 86400,
144 - 'currenttimestamp' => 3600,
145 - 'localtimestamp' => 3600,
146 - 'pagesinnamespace' => 3600,
147 - 'numberofadmins' => 3600,
148 - 'numberofviews' => 3600,
149 - 'numberingroup' => 3600,
 111+ 'currentmonth' => 86400,
 112+ 'currentmonth1' => 86400,
 113+ 'currentmonthname' => 86400,
 114+ 'currentmonthnamegen' => 86400,
 115+ 'currentmonthabbrev' => 86400,
 116+ 'currentday' => 3600,
 117+ 'currentday2' => 3600,
 118+ 'currentdayname' => 3600,
 119+ 'currentyear' => 86400,
 120+ 'currenttime' => 3600,
 121+ 'currenthour' => 3600,
 122+ 'localmonth' => 86400,
 123+ 'localmonth1' => 86400,
 124+ 'localmonthname' => 86400,
 125+ 'localmonthnamegen' => 86400,
 126+ 'localmonthabbrev' => 86400,
 127+ 'localday' => 3600,
 128+ 'localday2' => 3600,
 129+ 'localdayname' => 3600,
 130+ 'localyear' => 86400,
 131+ 'localtime' => 3600,
 132+ 'localhour' => 3600,
 133+ 'numberofarticles' => 3600,
 134+ 'numberoffiles' => 3600,
 135+ 'numberofedits' => 3600,
 136+ 'currentweek' => 3600,
 137+ 'currentdow' => 3600,
 138+ 'localweek' => 3600,
 139+ 'localdow' => 3600,
 140+ 'numberofusers' => 3600,
 141+ 'numberofactiveusers' => 3600,
 142+ 'numberofpages' => 3600,
 143+ 'currentversion' => 86400,
 144+ 'currenttimestamp' => 3600,
 145+ 'localtimestamp' => 3600,
 146+ 'pagesinnamespace' => 3600,
 147+ 'numberofadmins' => 3600,
 148+ 'numberofviews' => 3600,
 149+ 'numberingroup' => 3600,
 150+ 'numberofcontribs' => 3600,
150151 );
151152
152153 static public $mDoubleUnderscoreIDs = array(
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -340,6 +340,7 @@
341341 'index' => array( 1, '__INDEX__' ),
342342 'noindex' => array( 1, '__NOINDEX__' ),
343343 'numberingroup' => array( 1, 'NUMBERINGROUP', 'NUMINGROUP' ),
 344+ 'numberofcontribs' => array( 1, 'NUMBEROFCONTRIBS', 'USERCONTRIBS', 'NUMOFCONTRIBS' ),
344345 'staticredirect' => array( 1, '__STATICREDIRECT__' ),
345346 'protectionlevel' => array( 1, 'PROTECTIONLEVEL' ),
346347 'formatdate' => array( 0, 'formatdate', 'dateformat' ),
Index: trunk/phase3/RELEASE-NOTES
——@@ -82,6 +82,8 @@
8383 * (bug 11484) Added ISO speed rating to default collapsed EXIF metadata view
8484 * (bug 18958) Added ability to disable entire variant conversion engine
8585 per user preferences
 86+* (bug 19012) Introduce {{NUMBEROFCONTRIBS}} variable to display the number
 87+ of contributions a user has.
8688
8789 === Bug fixes in 1.16 ===
8890

Follow-up revisions

RevisionCommit summaryAuthorDate
r51253Some followup to r51200:...mrzman18:55, 31 May 2009
r51529Revert r51200, r51253, r51254, addition of {{NUMBEROFCONTRIBS:...}} parser fu...tstarling02:49, 6 June 2009

Comments

#Comment by Werdna (talk | contribs)   19:00, 31 May 2009

I don't like the look of this:

  1. No provided use case.
  2. Will cause cache inconsistency.
  3. Poorly-named, should be called usereditcount or something, we don't want to introduce more terminology inconsistencies.
  4. New parser functions should have a hash.
#Comment by Tim Starling (talk | contribs)   02:45, 6 June 2009

Some have said that there is a big difference between edits and contributions, and that one should not focus excessively on edit count since there are many other ways to contribute to the project. In this sense, the parser function most certainly shows edit count, not contribution count.

And along the same lines, I can easily imagine that placing this in the English Wikipedia's user tool templates (as the author surely wants to do) would only increase the fascination with this inaccurate measure of user contributions.

As for the code quality, the direct use of member variables is incorrect, accessors should be used instead. The vertical alignment in MagicWord.php is against current style guidelines, and introducing three different aliases for a parser function unnecessarily crowds the namespace.

Status & tagging log