r79018 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79017‎ | r79018 | r79019 >
Date:19:21, 26 December 2010
Author:platonides
Status:resolved (Comments)
Tags:
Comment:
Store the options used by the parsing in ParserOutput, per r70783 CR.
Modified paths:
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserCache.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOptions.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOutput.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
——@@ -22,7 +22,7 @@
2323 * produces altered wiki markup.
2424 * preprocess()
2525 * removes HTML comments and expands templates
26 - * cleanSig()
 26+ * cleanSig() / cleanSigInSig()
2727 * Cleans a signature before saving it to preferences
2828 * extractSections()
2929 * Extracts sections from an article for section editing
——@@ -192,6 +192,7 @@
193193 $this->firstCallInit();
194194 }
195195 $this->mOutput = new ParserOutput;
 196+ $this->mOptions->registerWatcher( array( $this->mOutput, 'recordOption' ) );
196197 $this->mAutonumber = 0;
197198 $this->mLastSection = '';
198199 $this->mDTopen = false;
——@@ -268,12 +269,11 @@
269270 wfProfileIn( __METHOD__ );
270271 wfProfileIn( $fname );
271272
 273+ $this->mOptions = $options;
272274 if ( $clearState ) {
273275 $this->clearState();
274276 }
275277
276 - $options->resetUsage();
277 - $this->mOptions = $options;
278278 $this->setTitle( $title ); # Page title has to be set for the pre-processor
279279
280280 $oldRevisionId = $this->mRevisionId;
——@@ -465,10 +465,9 @@
466466 */
467467 function preprocess( $text, $title, $options, $revid = null ) {
468468 wfProfileIn( __METHOD__ );
 469+ $this->mOptions = $options;
469470 $this->clearState();
470471 $this->setOutputType( self::OT_PREPROCESS );
471 - $options->resetUsage();
472 - $this->mOptions = $options;
473472 $this->setTitle( $title );
474473 if ( $revid !== null ) {
475474 $this->mRevisionId = $revid;
——@@ -489,10 +488,9 @@
490489 */
491490 public function getPreloadText( $text, $title, $options ) {
492491 # Parser (re)initialisation
 492+ $this->mOptions = $options;
493493 $this->clearState();
494494 $this->setOutputType( self::OT_PLAIN );
495 - $options->resetUsage();
496 - $this->mOptions = $options;
497495 $this->setTitle( $title );
498496
499497 $flags = PPFrame::NO_ARGS | PPFrame::NO_TEMPLATES;
——@@ -4029,7 +4027,6 @@
40304028 * @return String: the altered wiki markup
40314029 */
40324030 public function preSaveTransform( $text, Title $title, $user, $options, $clearState = true ) {
4033 - $options->resetUsage();
40344031 $this->mOptions = $options;
40354032 $this->setTitle( $title );
40364033 $this->setUser( $user );
——@@ -4211,9 +4208,9 @@
42124209 function cleanSig( $text, $parsing = false ) {
42134210 if ( !$parsing ) {
42144211 global $wgTitle;
 4212+ $this->mOptions = new ParserOptions;
42154213 $this->clearState();
42164214 $this->setTitle( $wgTitle );
4217 - $this->mOptions = new ParserOptions;
42184215 $this->setOutputType = self::OT_PREPROCESS;
42194216 }
42204217
——@@ -4861,9 +4858,9 @@
48624859 */
48634860 private function extractSections( $text, $section, $mode, $newText='' ) {
48644861 global $wgTitle;
 4862+ $this->mOptions = new ParserOptions;
48654863 $this->clearState();
48664864 $this->setTitle( $wgTitle ); # not generally used but removes an ugly failure mode
4867 - $this->mOptions = new ParserOptions;
48684865 $this->setOutputType( self::OT_PLAIN );
48694866 $outText = '';
48704867 $frame = $this->getPreprocessor()->newFrame();
——@@ -5143,13 +5140,13 @@
51445141 * strip/replaceVariables/unstrip for preprocessor regression testing
51455142 */
51465143 function testSrvus( $text, $title, $options, $outputType = self::OT_HTML ) {
 5144+ $this->mOptions = $options;
51475145 $this->clearState();
51485146 if ( !$title instanceof Title ) {
51495147 $title = Title::newFromText( $title );
51505148 }
51515149 $this->mTitle = $title;
51525150 $options->resetUsage();
5153 - $this->mOptions = $options;
51545151 $this->setOutputType( $outputType );
51555152 $text = $this->replaceVariables( $text );
51565153 $text = $this->mStripState->unstripBoth( $text );
Index: trunk/phase3/includes/parser/ParserOutput.php
——@@ -123,6 +123,7 @@
124124 $mProperties = array(), # Name/value pairs to be cached in the DB
125125 $mTOCHTML = ''; # HTML of the TOC
126126 private $mIndexPolicy = ''; # 'index' or 'noindex'? Any other value will result in no change.
 127+ private $mAccessedOptions = null; # List of ParserOptions (stored in the keys)
127128
128129 function __construct( $text = '', $languageLinks = array(), $categoryLinks = array(),
129130 $containsOldMagic = false, $titletext = '' )
——@@ -327,4 +328,25 @@
328329 }
329330 return $this->mProperties;
330331 }
 332+
 333+
 334+ /**
 335+ * Returns the options from its ParserOptions which have been taken
 336+ * into account to produce this output or false if not available.
 337+ * @return mixed Array/false
 338+ */
 339+ public function getUsedOptions() {
 340+ if ( !isset( $this->mAccessedOptions ) ) {
 341+ return false;
 342+ }
 343+ return array_keys( $this->mAccessedOptions );
 344+ }
 345+
 346+ /**
 347+ * Callback passed by the Parser to the ParserOptions to keep track of which options are used.
 348+ * @access private
 349+ */
 350+ function recordOption( $option ) {
 351+ $this->mAccessedOptions[$option] = true;
 352+ }
331353 }
Index: trunk/phase3/includes/parser/ParserCache.php
——@@ -170,7 +170,7 @@
171171 $now = wfTimestampNow();
172172
173173 $optionsKey = new CacheTime;
174 - $optionsKey->mUsedOptions = $popts->usedOptions();
 174+ $optionsKey->mUsedOptions = $parserOutput->getUsedOptions();
175175 $optionsKey->updateCacheExpiry( $expire );
176176
177177 $optionsKey->setCacheTime( $now );
Index: trunk/phase3/includes/parser/ParserOptions.php
——@@ -49,16 +49,16 @@
5050
5151 var $mExtraKey = ''; # Extra key that should be present in the caching key.
5252
53 - protected $accessedOptions;
 53+ protected $onAccessCallback = null;
5454
5555 function getUseDynamicDates() { return $this->mUseDynamicDates; }
5656 function getInterwikiMagic() { return $this->mInterwikiMagic; }
5757 function getAllowExternalImages() { return $this->mAllowExternalImages; }
5858 function getAllowExternalImagesFrom() { return $this->mAllowExternalImagesFrom; }
5959 function getEnableImageWhitelist() { return $this->mEnableImageWhitelist; }
60 - function getEditSection() { $this->accessedOptions['editsection'] = true;
 60+ function getEditSection() { $this->optionUsed('editsection');
6161 return $this->mEditSection; }
62 - function getNumberHeadings() { $this->accessedOptions['numberheadings'] = true;
 62+ function getNumberHeadings() { $this->optionUsed('numberheadings');
6363 return $this->mNumberHeadings; }
6464 function getAllowSpecialInclusion() { return $this->mAllowSpecialInclusion; }
6565 function getTidy() { return $this->mTidy; }
——@@ -73,14 +73,14 @@
7474 function getEnableLimitReport() { return $this->mEnableLimitReport; }
7575 function getCleanSignatures() { return $this->mCleanSignatures; }
7676 function getExternalLinkTarget() { return $this->mExternalLinkTarget; }
77 - function getMath() { $this->accessedOptions['math'] = true;
 77+ function getMath() { $this->optionUsed('math');
7878 return $this->mMath; }
79 - function getThumbSize() { $this->accessedOptions['thumbsize'] = true;
 79+ function getThumbSize() { $this->optionUsed('thumbsize');
8080 return $this->mThumbSize; }
8181
8282 function getIsPreview() { return $this->mIsPreview; }
8383 function getIsSectionPreview() { return $this->mIsSectionPreview; }
84 - function getIsPrintable() { $this->accessedOptions['printable'] = true;
 84+ function getIsPrintable() { $this->optionUsed('printable');
8585 return $this->mIsPrintable; }
8686 function getUser() { return $this->mUser; }
8787
——@@ -92,7 +92,7 @@
9393 }
9494
9595 function getDateFormat() {
96 - $this->accessedOptions['dateformat'] = true;
 96+ $this->optionUsed('dateformat');
9797 if ( !isset( $this->mDateFormat ) ) {
9898 $this->mDateFormat = $this->mUser->getDatePreference();
9999 }
——@@ -112,7 +112,7 @@
113113 * producing inconsistent tables (Bug 14404).
114114 */
115115 function getUserLang() {
116 - $this->accessedOptions['userlang'] = true;
 116+ $this->optionUsed('userlang');
117117 return $this->mUserLang;
118118 }
119119
——@@ -211,17 +211,20 @@
212212 }
213213
214214 /**
215 - * Returns the options from this ParserOptions which have been used.
 215+ * Registers a callback for tracking which ParserOptions which are used.
 216+ * This is a private API with the parser.
216217 */
217 - public function usedOptions() {
218 - return array_keys( $this->accessedOptions );
 218+ function registerWatcher( $callback ) {
 219+ $this->onAccessCallback = $callback;
219220 }
220221
221222 /**
222 - * Resets the memory of options usage.
 223+ * Called when an option is accessed.
223224 */
224 - public function resetUsage() {
225 - $this->accessedOptions = array();
 225+ protected function optionUsed( $optionName ) {
 226+ if ( $this->onAccessCallback ) {
 227+ call_user_func( $this->onAccessCallback, $optionName );
 228+ }
226229 }
227230
228231 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r79129MFT r78011 r78014 r78015 r78016 r78099 r78117 r78161 r78170 r78172 r78199 r78......platonides19:58, 28 December 2010
r80816Remove a couple of calls to ParserOptions::resetUsage(), missed on its remova...platonides15:56, 23 January 2011
r81737Tweak for r79018: fatal errors due to something calling parser clearState whe...brion13:30, 8 February 2011
r81738Tweak for r79018: fatal errors due to something calling parser clearState whe...brion13:31, 8 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70783Use only the page relevant pieces in the parser cache key. Eg. two users with...platonides21:53, 9 August 2010

Comments

#Comment by Catrope (talk | contribs)   22:19, 29 December 2010

Ideally this'd allow for multiple watch callbacks.

#Comment by Platonides (talk | contribs)   22:22, 29 December 2010

I considered it, but I'm not clear what should be the behavior if someone calls $wgParser->parse() again with clearState = false. We should probably keep with such callback set. Which means we can't remove the callback at the end of parse()

#Comment by Tbleher (talk | contribs)   15:29, 23 January 2011

This change breaks extensions which call Parser::startExternalParse(), because startExternalParse() calls $options->resetUsage();, but resetUsage() was removed in this commit.

There's one other place where resetUsage is called, this should be fixed too.

#Comment by Platonides (talk | contribs)   15:56, 23 January 2011

Fixed in r80816.

#Comment by Tbleher (talk | contribs)   16:16, 23 January 2011

Thanks, that was very fast! I can confirm that it works now.

Status & tagging log