r70842 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r70841‎ | r70842 | r70843 >
Date:19:57, 10 August 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Replace some instances of $wgUser passed to ParserOutput::get()/getKey() for $wgOut->parserOptions()
Modified paths:
  • /trunk/extensions/FlaggedRevs/api/ApiReview.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceInterface.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/diff/DifferenceInterface.php
——@@ -450,7 +450,7 @@
451451 }
452452 } elseif( $pCache ) {
453453 $article = new Article( $this->mTitle, 0 );
454 - $pOutput = ParserCache::singleton()->get( $article, $wgUser );
 454+ $pOutput = ParserCache::singleton()->get( $article, $wgOut->parserOptions() );
455455 if( $pOutput ) {
456456 $wgOut->addParserOutput( $pOutput );
457457 } else {
Index: trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php
——@@ -596,7 +596,7 @@
597597 $user, FlaggedArticle $article, Revision $rev,
598598 $templateIDs, $imageSHA1Keys, $stableDiff = false
599599 ) {
600 - global $wgRequest;
 600+ global $wgRequest, $wgOut;
601601 if ( $rev->isDeleted( Revision::DELETED_TEXT ) ) {
602602 return false; # The revision must be valid and public
603603 }
——@@ -689,7 +689,7 @@
690690 # Current version: try parser cache
691691 if ( $rev->isCurrent() ) {
692692 $parserCache = ParserCache::singleton();
693 - $pOutput = $parserCache->get( $article, $user );
 693+ $pOutput = $parserCache->get( $article, $wgOut->parserOptions() );
694694 }
695695 # Otherwise (or on cache miss), parse the rev text...
696696 if ( !$pOutput || !isset( $pOutput->fr_fileSHA1Keys ) ) {
Index: trunk/extensions/FlaggedRevs/api/ApiReview.php
——@@ -33,7 +33,7 @@
3434 * except that it generates the template and image parameters itself.
3535 */
3636 public function execute() {
37 - global $wgUser;
 37+ global $wgUser, $wgOut;
3838 $params = $this->extractRequestParams();
3939 // Check basic permissions
4040 if ( !$wgUser->isAllowed( 'review' ) ) {
——@@ -72,7 +72,7 @@
7373 $article = new FlaggedArticle( $title, $revid );
7474 if ( $rev->isCurrent() ) {
7575 $parserCache = ParserCache::singleton();
76 - $parserOutput = $parserCache->get( $article, $wgUser );
 76+ $parserOutput = $parserCache->get( $article, $wgOut->parserOptions() );
7777 }
7878 if ( !$parserOutput || !isset( $parserOutput->fr_fileSHA1Keys ) ) {
7979 // Miss, we have to reparse the page

Comments

#Comment by Platonides (talk | contribs)   14:40, 11 August 2010

I think that at least on some of them it should be $article->getParserOptions(), not $wgOut->parserOptions(). I'm not sure what's the difference between the two. I suspect it was in OutputPage, then moved into Article, leaving deprecated functions behind.

#Comment by 😂 (talk | contribs)   14:27, 8 September 2010

$article->getParserOptions() will unconditionally turn Tidy on and enable the limit report. $wgOut->parserOptions() doesn't explicitly set anything.

Both are using $wgUser.

#Comment by Platonides (talk | contribs)   22:34, 8 September 2010

However, $wgOut->parserOptions() defines itself as "the ParserOptions object to use for wikitext parsing".

#Comment by 😂 (talk | contribs)   22:35, 8 September 2010

Well isn't that nice and informative.

What on earth else would you use ParserOptions to parse?

#Comment by Platonides (talk | contribs)   22:31, 9 September 2010

You should ask ialex what he meant when documenting it a such in r61690 ;) (although you oked it).

I think that this usages should (some/all) should create a new ParserOptions instead of calling that useless parserOptions(). Specially since r70783.

And DifferenceInterface::renderNewRevision() should be callign $article->getParserOutput() not calling the parsercache directly. I will have been cached with Article::getParserOptions() not OutputPage::parserOptions(). They don't produce different parser cache keys now, but perhaps they will in the future.

#Comment by 😂 (talk | contribs)   15:13, 12 September 2010

$wgOut->parserOptions() is used a lot of places :( Maybe update callers (there's a lot in extensions) to do something more sane and then start throwing wfDeprecated() on it?

$article->getParserOptions() isn't a drop-in replacement for DIY with $wgOut->parserOptions() yet, since the latter sets some specific options (no edit section links, etc) first. Thus, we can't use $article->getParserOutput() in renderNewRevision() just yet. Thoughts?

#Comment by Catrope (talk | contribs)   23:42, 22 December 2010

After some thorough research through about a dozen levels of indirection, it turns out the (old?) behavior of passing a User object instead of a ParserOptions object is identical to that of calling $wgOut->parserOptions(), therefore this revision shouldn't actually change any behavior, therefore I'm marking it as OK.

Status & tagging log