r59787 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r59786‎ | r59787 | r59788 >
Date:14:14, 7 December 2009
Author:thomasv
Status:resolved (Comments)
Tags:
Comment:
create a fresh parser object, as suggested by brion in r56878
Modified paths:
  • /trunk/extensions/ProofreadPage/ProofreadPage.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ProofreadPage/ProofreadPage.php
——@@ -285,21 +285,18 @@
286286 /**
287287 * Return the ordered list of links to ns-0 from an index page
288288 */
289 -function pr_parse_index_links( $index_title, $parser ){
 289+function pr_parse_index_links( $index_title ){
290290
291291 $rev = Revision::newFromTitle( $index_title );
292292 $text = $rev->getText();
293 -
294 - # We use Parser::replaceVariables to expand templates
295 - # However this method has a side effect on wgParser->mOutput->mTemplates,
296 - # To avoid this, we instanciate a temporary ParserOutput object
297 - $saved_output = $parser->mOutput;
298 - $parser->mOutput = new ParserOutput;
 293+ # Instanciate a new parser object to avoid side effects of $parser->replaceVariables
 294+ $parser = new Parser;
 295+ $parser->clearState();
 296+ $parser->mOptions = new ParserOptions();
 297+ $parser->setTitle( $index_title );
299298 $rtext = $parser->replaceVariables( $text );
300 - $parser->mOutput = $saved_output;
301299 $text_links_pattern = "/\[\[([^:\|]*?)(\|(.*?)|)\]\]/i";
302300 preg_match_all( $text_links_pattern, $rtext, $text_links, PREG_PATTERN_ORDER );
303 -
304301 return $text_links;
305302 }
306303
——@@ -727,7 +724,6 @@
728725 $out = '';
729726
730727 list( $links, $params, $attributes ) = pr_parse_index( $index_title );
731 - $text_links = pr_parse_index_links( $index_title, $parser );
732728
733729 if( $from || $to ) {
734730
——@@ -810,6 +806,7 @@
811807 }
812808
813809 if( $header ) {
 810+ $text_links = pr_parse_index_links( $index_title );
814811 $h_out = '{{:MediaWiki:Proofreadpage_header_template';
815812 $h_out .= "|value=$header";
816813 //find next and previous pages in list

Follow-up revisions

RevisionCommit summaryAuthorDate
r59872follow up to r59787 : preprocess, cache parser instancethomasv08:42, 9 December 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r56878fix: Parser::replaceVariables() has a side effectthomasv13:33, 24 September 2009

Comments

#Comment by Tim Starling (talk | contribs)   00:18, 9 December 2009

Creating a parser like this is OK, but you should cache the object since it takes ~12ms on the first call to set it up. It's not correct to call clearState() or setTitle() or to set mOptions from external code. Instead you should use preprocess() which does that for you. Preprocess is very similar to replaceVariables() except that it avoids leaving strip markers in the output, and it has the appropriate setup and shutdown code.

Status & tagging log