r100227 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r100226‎ | r100227 | r100228 >
Date:14:16, 19 October 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
* Changed ParserOptions to store a Language object instead of only a string, avoids object -> string -> object conversion
* ParserOptions::getUserLang() will still return a string for compatibility, added ParserOptions::getUserLangObj() to get the object
* Added ParserOptions::newFromUserAndLang() and ParserOptions::newFromContext() to easily get a ParserOptions object when a context is available or when someone wants to force the language
* Updated OutputPage and Preferences to use newFromContext() and WikiPage to use newFromUserAndLang()
* ParserOptions::setUserLang() still accepts either a string or a Language object, but changed the calls to pass an object instead of a string
* Changed Parser::getFunctionLang() to return the Language object from ParserOptions when parsing interface messages rather than $wgLang directly and updated the documentation to say that $wgLang should not be used directly (as $wgUser, $wgTitle and $wgRequest)
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Preferences.php (modified) (history)
  • /trunk/phase3/includes/WikiPage.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.php (modified) (history)
  • /trunk/phase3/includes/parser/CoreParserFunctions.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOptions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
——@@ -32,9 +32,9 @@
3333 * Removes <noinclude> sections, and <includeonly> tags.
3434 *
3535 * Globals used:
36 - * objects: $wgLang, $wgContLang
 36+ * object: $wgContLang
3737 *
38 - * NOT $wgUser or $wgTitle or $wgRequest. Keep them away!
 38+ * NOT $wgUser or $wgTitle or $wgRequest or $wgLang. Keep them away!
3939 *
4040 * settings:
4141 * $wgUseDynamicDates*, $wgInterwikiMagic*,
——@@ -698,8 +698,7 @@
699699 if ( $target !== null ) {
700700 return $target;
701701 } elseif( $this->mOptions->getInterfaceMessage() ) {
702 - global $wgLang;
703 - return $wgLang;
 702+ return $this->mOptions->getUserLangObj();
704703 } elseif( is_null( $this->mTitle ) ) {
705704 throw new MWException( __METHOD__.': $this->mTitle is null' );
706705 }
——@@ -3237,7 +3236,7 @@
32383237 $context->setTitle( $title );
32393238 $context->setRequest( new FauxRequest( $pageArgs ) );
32403239 $context->setUser( $this->getUser() );
3241 - $context->setLang( Language::factory( $this->mOptions->getUserLang() ) );
 3240+ $context->setLang( $this->mOptions->getUserLangObj() );
32423241 $ret = SpecialPageFactory::capturePath( $title, $context );
32433242 if ( $ret ) {
32443243 $text = $context->getOutput()->getHTML();
Index: trunk/phase3/includes/parser/CoreParserFunctions.php
——@@ -97,7 +97,7 @@
9898 static function intFunction( $parser, $part1 = '' /*, ... */ ) {
9999 if ( strval( $part1 ) !== '' ) {
100100 $args = array_slice( func_get_args(), 2 );
101 - $message = wfMessage( $part1, $args )->inLanguage( $parser->getOptions()->getUserLang() )->plain();
 101+ $message = wfMessage( $part1, $args )->inLanguage( $parser->getOptions()->getUserLangObj() )->plain();
102102 $message = $parser->replaceVariables( $message ); // like MessageCache::transform()
103103 return $message;
104104 } else {
Index: trunk/phase3/includes/parser/ParserOptions.php
——@@ -41,7 +41,7 @@
4242 var $mMath; # User math preference (as integer)
4343 var $mThumbSize; # Thumb size preferred by the user.
4444 private $mStubThreshold; # Maximum article size of an article to be marked as "stub"
45 - var $mUserLang; # Language code of the User language.
 45+ var $mUserLang; # Language object of the User language.
4646
4747 /**
4848 * @var User
——@@ -119,12 +119,23 @@
120120 * You shouldn't use this. Really. $parser->getFunctionLang() is all you need.
121121 * Using this fragments the cache and is discouraged. Yes, {{int: }} uses this,
122122 * producing inconsistent tables (Bug 14404).
 123+ *
 124+ * @return Language object
 125+ * @since 1.19
 126+ */
 127+ function getUserLangObj() {
 128+ $this->optionUsed( 'userlang' );
 129+ return $this->mUserLang;
 130+ }
 131+
 132+ /**
 133+ * Same as getUserLangObj() but returns a string instead.
 134+ *
123135 * @return String Language code
124136 * @since 1.17
125137 */
126138 function getUserLang() {
127 - $this->optionUsed( 'userlang' );
128 - return $this->mUserLang;
 139+ return $this->getUserLangObj()->getCode();
129140 }
130141
131142 function setUseDynamicDates( $x ) { return wfSetVar( $this->mUseDynamicDates, $x ); }
——@@ -153,8 +164,8 @@
154165 function setExternalLinkTarget( $x ) { return wfSetVar( $this->mExternalLinkTarget, $x ); }
155166 function setMath( $x ) { return wfSetVar( $this->mMath, $x ); }
156167 function setUserLang( $x ) {
157 - if ( $x instanceof Language ) {
158 - $x = $x->getCode();
 168+ if ( is_string( $x ) ) {
 169+ $x = Language::factory( $x );
159170 }
160171 return wfSetVar( $this->mUserLang, $x );
161172 }
——@@ -173,42 +184,63 @@
174185 $this->mExtraKey .= '!' . $key;
175186 }
176187
177 - function __construct( $user = null ) {
178 - $this->initialiseFromUser( $user );
 188+ function __construct( $user = null, $lang = null ) {
 189+ if ( $user === null ) {
 190+ global $wgUser;
 191+ if ( $wgUser === null ) {
 192+ $user = new User;
 193+ } else {
 194+ $user = $wgUser;
 195+ }
 196+ }
 197+ if ( $lang === null ) {
 198+ global $wgLang;
 199+ $lang = $wgLang;
 200+ }
 201+ $this->initialiseFromUser( $user, $lang );
179202 }
180203
181204 /**
182 - * Get parser options
 205+ * Get a ParserOptions object from a given user.
 206+ * Language will be taken from $wgLang.
183207 *
184208 * @param $user User object
185209 * @return ParserOptions object
186210 */
187 - static function newFromUser( $user ) {
 211+ public static function newFromUser( $user ) {
188212 return new ParserOptions( $user );
189213 }
190214
 215+ /**
 216+ * Get a ParserOptions object from a given user and language
 217+ *
 218+ * @param $user User object
 219+ * @param $lang Language object
 220+ * @return ParserOptions object
 221+ */
 222+ public static function newFromUserAndLang( User $user, Language $lang ) {
 223+ return new ParserOptions( $user, $lang );
 224+ }
 225+
 226+ /**
 227+ * Get a ParserOptions object from a IContextSource object
 228+ *
 229+ * @param $context IContextSource object
 230+ * @return ParserOptions object
 231+ */
 232+ public static function newFromContext( IContextSource $context ) {
 233+ return new ParserOptions( $context->getUser(), $context->getLang() );
 234+ }
 235+
191236 /** Get user options */
192 - function initialiseFromUser( $userInput ) {
193 - global $wgUseDynamicDates, $wgInterwikiMagic, $wgAllowExternalImages;
194 - global $wgAllowExternalImagesFrom, $wgEnableImageWhitelist, $wgAllowSpecialInclusion, $wgMaxArticleSize;
195 - global $wgMaxPPNodeCount, $wgMaxTemplateDepth, $wgMaxPPExpandDepth, $wgCleanSignatures;
196 - global $wgExternalLinkTarget, $wgLang;
 237+ private function initialiseFromUser( $user, $lang ) {
 238+ global $wgUseDynamicDates, $wgInterwikiMagic, $wgAllowExternalImages,
 239+ $wgAllowExternalImagesFrom, $wgEnableImageWhitelist, $wgAllowSpecialInclusion,
 240+ $wgMaxArticleSize, $wgMaxPPNodeCount, $wgMaxTemplateDepth, $wgMaxPPExpandDepth,
 241+ $wgCleanSignatures, $wgExternalLinkTarget;
197242
198243 wfProfileIn( __METHOD__ );
199244
200 - if ( !$userInput ) {
201 - global $wgUser;
202 - if ( isset( $wgUser ) ) {
203 - $user = $wgUser;
204 - } else {
205 - $user = new User;
206 - }
207 - } else {
208 - $user =& $userInput;
209 - }
210 -
211 - $this->mUser = $user;
212 -
213245 $this->mUseDynamicDates = $wgUseDynamicDates;
214246 $this->mInterwikiMagic = $wgInterwikiMagic;
215247 $this->mAllowExternalImages = $wgAllowExternalImages;
——@@ -222,11 +254,12 @@
223255 $this->mCleanSignatures = $wgCleanSignatures;
224256 $this->mExternalLinkTarget = $wgExternalLinkTarget;
225257
 258+ $this->mUser = $user;
226259 $this->mNumberHeadings = $user->getOption( 'numberheadings' );
227260 $this->mMath = $user->getOption( 'math' );
228261 $this->mThumbSize = $user->getOption( 'thumbsize' );
229262 $this->mStubThreshold = $user->getStubThreshold();
230 - $this->mUserLang = $wgLang->getCode();
 263+ $this->mUserLang = $lang;
231264
232265 wfProfileOut( __METHOD__ );
233266 }
——@@ -312,7 +345,7 @@
313346 }
314347
315348 if ( in_array( 'userlang', $forOptions ) ) {
316 - $confstr .= '!' . $this->mUserLang;
 349+ $confstr .= '!' . $this->mUserLang->getCode();
317350 } else {
318351 $confstr .= '!*';
319352 }
Index: trunk/phase3/includes/OutputPage.php
——@@ -1249,7 +1249,7 @@
12501250 */
12511251 public function parserOptions( $options = null ) {
12521252 if ( !$this->mParserOptions ) {
1253 - $this->mParserOptions = new ParserOptions;
 1253+ $this->mParserOptions = ParserOptions::newFromContext( $this->getContext() );
12541254 $this->mParserOptions->setEditSection( false );
12551255 }
12561256 return wfSetVar( $this->mParserOptions, $options );
Index: trunk/phase3/includes/installer/Installer.php
——@@ -1202,7 +1202,7 @@
12031203 */
12041204 public function setParserLanguage( $lang ) {
12051205 $this->parserOptions->setTargetLanguage( $lang );
1206 - $this->parserOptions->setUserLang( $lang->getCode() );
 1206+ $this->parserOptions->setUserLang( $lang );
12071207 }
12081208
12091209 /**
Index: trunk/phase3/includes/WikiPage.php
——@@ -1950,7 +1950,7 @@
19511951 * Returns a stdclass with source, pst and output members
19521952 */
19531953 public function prepareTextForEdit( $text, $revid = null, User $user = null ) {
1954 - global $wgParser, $wgUser;
 1954+ global $wgParser, $wgContLang, $wgUser;
19551955 $user = is_null( $user ) ? $wgUser : $user;
19561956 // @TODO fixme: check $user->getId() here???
19571957 if ( $this->mPreparedEdit
——@@ -1961,7 +1961,7 @@
19621962 return $this->mPreparedEdit;
19631963 }
19641964
1965 - $popts = ParserOptions::newFromUser( $user );
 1965+ $popts = ParserOptions::newFromUserAndLang( $user, $wgContLang );
19661966 wfRunHooks( 'ArticlePrepareTextForEdit', array( $this, $popts ) );
19671967
19681968 $edit = (object)array();
——@@ -2507,12 +2507,11 @@
25082508 * @return ParserOptions
25092509 */
25102510 public function makeParserOptions( $user ) {
2511 - global $wgLanguageCode;
 2511+ global $wgContLang;
25122512 if ( $user instanceof User ) { // settings per user (even anons)
25132513 $options = ParserOptions::newFromUser( $user );
25142514 } else { // canonical settings
2515 - $options = ParserOptions::newFromUser( new User );
2516 - $options->setUserLang( $wgLanguageCode ); # Must be set explicitily
 2515+ $options = ParserOptions::newFromUserAndLang( new User, $wgContLang );
25172516 }
25182517 $options->enableLimitReport(); // show inclusion/loop reports
25192518 $options->setTidy( true ); // fix bad HTML
Index: trunk/phase3/includes/Preferences.php
——@@ -305,7 +305,7 @@
306306 }
307307
308308 // show a preview of the old signature first
309 - $oldsigWikiText = $wgParser->preSaveTransform( "~~~", $context->getTitle(), $user, new ParserOptions );
 309+ $oldsigWikiText = $wgParser->preSaveTransform( "~~~", $context->getTitle(), $user, ParserOptions::newFromContext( $context ) );
310310 $oldsigHTML = $context->getOutput()->parseInline( $oldsigWikiText, true, true );
311311 $defaultPreferences['oldsig'] = array(
312312 'type' => 'info',

Follow-up revisions

RevisionCommit summaryAuthorDate
r100232Follow-up r100227:...ialex15:30, 19 October 2011

Comments

#Comment by Platonides (talk | contribs)   22:11, 20 October 2011

It seems to do the stated job, but... why?

#Comment by IAlex (talk | contribs)   05:03, 22 October 2011

As always, reducing the dependency on that global objects, even if their usage is hidden somewhere.

Status & tagging log