r29945 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r29944‎ | r29945 | r29946 >
Date:09:03, 19 January 2008
Author:tstarling
Status:old
Tags:
Comment:
* Eliminated message mode (OT_MSG), using OT_PREPROCESS instead. As proposed on wikitech-l.
* Fixed #tag behaviour in preprocess()
* Fixed #tag quote stripping regex
* Made MessageCache::getMessage() never transform its result, that is now left up to the caller.
* A few other minor changes
Modified paths:
  • /trunk/phase3/includes/CoreParserFunctions.php (modified) (history)
  • /trunk/phase3/includes/Defines.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/MessageCache.php (modified) (history)
  • /trunk/phase3/includes/Parser.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Defines.php
——@@ -268,8 +268,8 @@
269269 # Parameter to Parser::startExternalParse().
270270 define( 'OT_HTML', 1 );
271271 define( 'OT_WIKI', 2 );
272 -define( 'OT_MSG' , 3 );
273 -define( 'OT_PREPROCESS', 4 );
 272+define( 'OT_PREPROCESS', 3 );
 273+define( 'OT_MSG' , 3 ); // b/c alias for OT_PREPROCESS
274274
275275 # Flags for Parser::setFunctionHook
276276 define( 'SFH_NO_HASH', 1 );
Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -427,24 +427,12 @@
428428 function wfMsgGetKey( $key, $useDB, $forContent = false, $transform = true ) {
429429 global $wgParser, $wgContLang, $wgMessageCache, $wgLang;
430430
431 - /* <Vyznev> btw, is all that code in wfMsgGetKey() that check
432 - * if the message cache exists of not really necessary, or is
433 - * it just paranoia?
434 - * <TimStarling> Vyznev: it's probably not necessary
435 - * <TimStarling> I think I wrote it in an attempt to report DB
436 - * connection errors properly
437 - * <TimStarling> but eventually we gave up on using the
438 - * message cache for that and just hard-coded the strings
439 - * <TimStarling> it may have other uses, it's not mere paranoia
440 - */
441 -
442 - if ( is_object( $wgMessageCache ) )
443 - $transstat = $wgMessageCache->getTransform();
444 -
 431+ # If $wgMessageCache isn't initialised yet, try to return something sensible.
445432 if( is_object( $wgMessageCache ) ) {
446 - if ( ! $transform )
447 - $wgMessageCache->disableTransform();
448433 $message = $wgMessageCache->get( $key, $useDB, $forContent );
 434+ if ( $transform ) {
 435+ $message = $wgMessageCache->transform( $message );
 436+ }
449437 } else {
450438 if( $forContent ) {
451439 $lang = &$wgContLang;
——@@ -456,22 +444,13 @@
457445 # ISSUE: Should we try to handle "message/lang" here too?
458446 $key = str_replace( ' ' , '_' , $wgContLang->lcfirst( $key ) );
459447
460 - wfSuppressWarnings();
461448 if( is_object( $lang ) ) {
462449 $message = $lang->getMessage( $key );
463450 } else {
464451 $message = false;
465452 }
466 - wfRestoreWarnings();
467 -
468 - if ( $transform && strstr( $message, '{{' ) !== false ) {
469 - $message = $wgParser->transformMsg($message, $wgMessageCache->getParserOptions() );
470 - }
471453 }
472454
473 - if ( is_object( $wgMessageCache ) && ! $transform )
474 - $wgMessageCache->setTransform( $transstat );
475 -
476455 return $message;
477456 }
478457
Index: trunk/phase3/includes/MessageCache.php
——@@ -500,9 +500,6 @@
501501 if( $message === false ) {
502502 return '&lt;' . htmlspecialchars($key) . '&gt;';
503503 }
504 -
505 - # Replace brace tags
506 - $message = $this->transform( $message );
507504 return $message;
508505 }
509506
——@@ -584,12 +581,11 @@
585582 # Clone it and store it
586583 $this->mParser = clone $wgParser;
587584 }
588 - if ( !$this->mDisableTransform && $this->mParser ) {
 585+ if ( $this->mParser ) {
589586 if( strpos( $message, '{{' ) !== false ) {
590587 $popts = $this->getParserOptions();
591 - if ( $interface ) { $popts->setInterfaceMessage(true); }
 588+ $popts->setInterfaceMessage( $interface );
592589 $message = $this->mParser->transformMsg( $message, $popts );
593 - if ( $interface ) { $popts->setInterfaceMessage(false); }
594590 }
595591 }
596592 return $message;
——@@ -597,11 +593,13 @@
598594
599595 function disable() { $this->mDisable = true; }
600596 function enable() { $this->mDisable = false; }
601 - function disableTransform() { $this->mDisableTransform = true; }
602 - function enableTransform() { $this->mDisableTransform = false; }
603 - function setTransform( $x ) { $this->mDisableTransform = $x; }
604 - function getTransform() { return $this->mDisableTransform; }
605597
 598+ /** @deprecated */
 599+ function disableTransform() {}
 600+ function enableTransform() {}
 601+ function setTransform( $x ) {}
 602+ function getTransform() { return false; }
 603+
606604 /**
607605 * Add a message to the cache
608606 *
Index: trunk/phase3/includes/Parser.php
——@@ -15,15 +15,17 @@
1616 * (which in turn the browser understands, and can display).
1717 *
1818 * <pre>
19 - * There are four main entry points into the Parser class:
 19+ * There are five main entry points into the Parser class:
2020 * parse()
2121 * produces HTML output
2222 * preSaveTransform().
2323 * produces altered wiki markup.
24 - * transformMsg()
25 - * performs brace substitution on MediaWiki messages
2624 * preprocess()
2725 * removes HTML comments and expands templates
 26+ * cleanSig()
 27+ * Cleans a signature before saving it to preferences
 28+ * extractSections()
 29+ * Extracts sections from an article for section editing
2830 *
2931 * Globals used:
3032 * objects: $wgLang, $wgContLang
——@@ -247,7 +249,6 @@
248250 $this->ot = array(
249251 'html' => $ot == OT_HTML,
250252 'wiki' => $ot == OT_WIKI,
251 - 'msg' => $ot == OT_MSG,
252253 'pre' => $ot == OT_PREPROCESS,
253254 );
254255 }
——@@ -256,6 +257,9 @@
257258 * Set the context title
258259 */
259260 function setTitle( $t ) {
 261+ if ( !$t || $t instanceof FakeTitle ) {
 262+ $t = Title::newFromText( 'NO TITLE' );
 263+ }
260264 if ( strval( $t->getFragment() ) !== '' ) {
261265 # Strip the fragment to avoid various odd effects
262266 $this->mTitle = clone $t;
——@@ -453,9 +457,6 @@
454458 wfRunHooks( 'ParserBeforeStrip', array( &$this, &$text, &$this->mStripState ) );
455459 wfRunHooks( 'ParserAfterStrip', array( &$this, &$text, &$this->mStripState ) );
456460 $text = $this->replaceVariables( $text );
457 - if ( $this->mOptions->getRemoveComments() ) {
458 - $text = Sanitizer::removeHTMLcomments( $text );
459 - }
460461 $text = $this->mStripState->unstripBoth( $text );
461462 wfProfileOut( __METHOD__ );
462463 return $text;
——@@ -2581,7 +2582,7 @@
25822583 * self::PTD_FOR_INCLUSION Handle <noinclude>/<includeonly> as if the text is being
25832584 * included. Default is to assume a direct page view.
25842585 *
2585 - * The generated DOM tree must depend only on the input text, the flags, and $this->ot['msg'].
 2586+ * The generated DOM tree must depend only on the input text and the flags.
25862587 * The DOM tree must be the same in OT_HTML and OT_WIKI mode, to avoid a regression of bug 4899.
25872588 *
25882589 * Any flag added to the $flags parameter here, or any other parameter liable to cause a
——@@ -2598,47 +2599,24 @@
25992600 wfProfileIn( __METHOD__ );
26002601 wfProfileIn( __METHOD__.'-makexml' );
26012602
2602 - static $msgRules, $normalRules, $inclusionSupertags, $nonInclusionSupertags;
2603 - if ( !$msgRules ) {
2604 - $msgRules = array(
2605 - '{' => array(
2606 - 'end' => '}',
2607 - 'names' => array(
2608 - 2 => 'template',
2609 - ),
2610 - 'min' => 2,
2611 - 'max' => 2,
 2603+ $rules = array(
 2604+ '{' => array(
 2605+ 'end' => '}',
 2606+ 'names' => array(
 2607+ 2 => 'template',
 2608+ 3 => 'tplarg',
26122609 ),
2613 - '[' => array(
2614 - 'end' => ']',
2615 - 'names' => array( 2 => null ),
2616 - 'min' => 2,
2617 - 'max' => 2,
2618 - )
2619 - );
2620 - $normalRules = array(
2621 - '{' => array(
2622 - 'end' => '}',
2623 - 'names' => array(
2624 - 2 => 'template',
2625 - 3 => 'tplarg',
2626 - ),
2627 - 'min' => 2,
2628 - 'max' => 3,
2629 - ),
2630 - '[' => array(
2631 - 'end' => ']',
2632 - 'names' => array( 2 => null ),
2633 - 'min' => 2,
2634 - 'max' => 2,
2635 - )
2636 - );
2637 - }
2638 - if ( $this->ot['msg'] ) {
2639 - $rules = $msgRules;
2640 - } else {
2641 - $rules = $normalRules;
2642 - }
 2610+ 'min' => 2,
 2611+ 'max' => 3,
 2612+ ),
 2613+ '[' => array(
 2614+ 'end' => ']',
 2615+ 'names' => array( 2 => null ),
 2616+ 'min' => 2,
 2617+ 'max' => 2,
 2618+ )
 2619+ );
 2620+
26432621 $forInclusion = $flags & self::PTD_FOR_INCLUSION;
26442622
26452623 $xmlishElements = $this->getStripList();
——@@ -2662,7 +2640,7 @@
26632641
26642642 $stack = new PPDStack;
26652643
2666 - $searchBase = implode( '', array_keys( $rules ) ) . '<';
 2644+ $searchBase = '[{<';
26672645 $revText = strrev( $text ); // For fast reverse searches
26682646
26692647 $i = 0; # Input pointer, starts out pointing to a pseudo-newline before the start
——@@ -2931,8 +2909,14 @@
29322910 if ( preg_match( "/\s*(={{$count}})/A", $revText, $m, 0, strlen( $text ) - $i ) ) {
29332911 if ( $i - strlen( $m[0] ) == $piece->startPos ) {
29342912 // This is just a single string of equals signs on its own line
2935 - // Divide by two and round down to create start and end delimiters
2936 - $count = intval( $count / 2 );
 2913+ // Replicate the doHeadings behaviour /={count}(.+)={count}/
 2914+ if ( $count < 3 ) {
 2915+ $count = 0;
 2916+ } elseif ( $count % 2 ) {
 2917+ $count = ( $count - 1 ) / 2;
 2918+ } else {
 2919+ $count = $count / 2 - 1;
 2920+ }
29372921 } else {
29382922 $count = min( strlen( $m[1] ), $count );
29392923 }
——@@ -3160,8 +3144,8 @@
31613145 *
31623146 * Note that the substitution depends on value of $mOutputType:
31633147 * OT_WIKI: only {{subst:}} templates
3164 - * OT_MSG: only magic variables
3165 - * OT_HTML: all templates and magic variables
 3148+ * OT_PREPROCESS: templates but not extension tags
 3149+ * OT_HTML: all templates and extension tags
31663150 *
31673151 * @param string $tex The text to transform
31683152 * @param PPFrame $frame Object describing the arguments passed to the template
——@@ -3733,6 +3717,15 @@
37343718 }
37353719 }
37363720 } else {
 3721+ if ( is_null( $attrText ) ) {
 3722+ $attrText = '';
 3723+ }
 3724+ if ( isset( $params['attributes'] ) ) {
 3725+ foreach ( $params['attributes'] as $attrName => $attrValue ) {
 3726+ $attrText .= ' ' . htmlspecialchars( $attrName ) . '="' .
 3727+ htmlspecialchars( $attrValue ) . '"';
 3728+ }
 3729+ }
37373730 if ( $content === null ) {
37383731 $output = "<$name$attrText/>";
37393732 } else {
——@@ -4244,7 +4237,10 @@
42454238 function cleanSig( $text, $parsing = false ) {
42464239 if ( !$parsing ) {
42474240 global $wgTitle;
4248 - $this->startExternalParse( $wgTitle, new ParserOptions(), OT_MSG );
 4241+ $this->clearState();
 4242+ $this->setTitle( $wgTitle );
 4243+ $this->mOptions = new ParserOptions;
 4244+ $this->setOutputType = OT_PREPROCESS;
42494245 }
42504246
42514247 # FIXME: regex doesn't respect extension tags or nowiki
——@@ -4291,16 +4287,11 @@
42924288 }
42934289
42944290 /**
4295 - * Transform a MediaWiki message by replacing magic variables.
 4291+ * Wrapper for preprocess()
42964292 *
4297 - * For some unknown reason, it also expands templates, but only to the
4298 - * first recursion level. This is wrong and broken, probably introduced
4299 - * accidentally during refactoring, but probably relied upon by thousands
4300 - * of users.
4301 - *
4302 - * @param string $text the text to transform
 4293+ * @param string $text the text to preprocess
43034294 * @param ParserOptions $options options
4304 - * @return string the text with variables substituted
 4295+ * @return string
43054296 * @public
43064297 */
43074298 function transformMsg( $text, $options ) {
——@@ -4316,18 +4307,8 @@
43174308 $executing = true;
43184309
43194310 wfProfileIn($fname);
 4311+ $text = $this->preprocess( $text, $wgTitle, $options );
43204312
4321 - if ( $wgTitle && !( $wgTitle instanceof FakeTitle ) ) {
4322 - $this->setTitle( $wgTitle );
4323 - } else {
4324 - $this->setTitle( Title::newFromText('msg') );
4325 - }
4326 - $this->mOptions = $options;
4327 - $this->setOutputType( OT_MSG );
4328 - $this->clearState();
4329 - $text = $this->replaceVariables( $text );
4330 - $text = $this->mStripState->unstripBoth( $text );
4331 -
43324313 $executing = false;
43334314 wfProfileOut($fname);
43344315 return $text;
——@@ -5415,22 +5396,16 @@
54165397 return $root;
54175398 }
54185399
5419 - if ( $this->parser->ot['html']
5420 - && ++$this->parser->mPPNodeCount > $this->parser->mOptions->mMaxPPNodeCount )
 5400+ if ( ++$this->parser->mPPNodeCount > $this->parser->mOptions->mMaxPPNodeCount )
54215401 {
5422 - return $this->parser->insertStripItem( '<!-- node-count limit exceeded -->' );
 5402+ return '<span class="error">Node-count limit exceeded</span>';
54235403 }
54245404
5425 - if ( is_array( $root ) ) {
 5405+ if ( is_array( $root ) || $root instanceof DOMNodeList ) {
54265406 $s = '';
54275407 foreach ( $root as $node ) {
54285408 $s .= $this->expand( $node, $flags );
54295409 }
5430 - } elseif ( $root instanceof DOMNodeList ) {
5431 - $s = '';
5432 - foreach ( $root as $node ) {
5433 - $s .= $this->expand( $node, $flags );
5434 - }
54355410 } elseif ( $root instanceof DOMNode ) {
54365411 if ( $root->nodeType == XML_TEXT_NODE ) {
54375412 $s = $root->nodeValue;
——@@ -5457,7 +5432,7 @@
54585433 $titles = $xpath->query( 'title', $root );
54595434 $title = $titles->item( 0 );
54605435 $parts = $xpath->query( 'part', $root );
5461 - if ( $flags & self::NO_ARGS || $this->parser->ot['msg'] ) {
 5436+ if ( $flags & self::NO_ARGS ) {
54625437 $s = '{{{' . $this->implodeWithFlags( '|', $flags, $title, $parts ) . '}}}';
54635438 } else {
54645439 $params = array( 'title' => $title, 'parts' => $parts, 'text' => 'FIXME' );
——@@ -5699,7 +5674,6 @@
57005675 }
57015676
57025677 function getArgument( $name ) {
5703 - wfDebug( __METHOD__." getting '$name'\n" );
57045678 $text = $this->getNumberedArgument( $name );
57055679 if ( $text === false ) {
57065680 $text = $this->getNamedArgument( $name );
Index: trunk/phase3/includes/CoreParserFunctions.php
——@@ -245,8 +245,8 @@
246246 $name = $frame->expand( $names->item( 0 ), PPFrame::STRIP_COMMENTS );
247247 $values = $xpath->query( 'value', $arg );
248248 $value = trim( $frame->expand( $values->item( 0 ) ) );
249 - if ( preg_match( '/^(?:"|\')(.*)(?:"|\')$/s', $value, $m ) ) {
250 - $value = $m[1];
 249+ if ( preg_match( '/^(?:["\'](.+)["\']|""|\'\')$/s', $value, $m ) ) {
 250+ $value = isset( $m[1] ) ? $m[1] : '';
251251 }
252252 $attributes[$name] = $value;
253253 }
——@@ -255,7 +255,8 @@
256256 $params = array(
257257 'name' => $tagName,
258258 'inner' => $inner,
259 - 'attributes' => $attributes
 259+ 'attributes' => $attributes,
 260+ 'close' => "</$tagName>",
260261 );
261262 return $parser->extensionSubstitution( $params, $frame );
262263 }

Status & tagging log