MediaWiki r61710 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r61709‎ | r61710 (on ViewVC)‎ | r61711 >
Date:11:58, 30 January 2010
Author:conrad
Status:deferred (Comments)
Tags:
Comment:
bug 22297 - "syntax for substitution that doesn't break transclusion"
Adds "safesubst:$1" that works similarly to "subst:$1"
(relevant to bug 5453, bug 16714, bug 4484)
Modified paths:

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
===================================================================
--- trunk/phase3/includes/parser/Parser.php	(revision 61709)
+++ trunk/phase3/includes/parser/Parser.php	(revision 61710)
@@ -91,9 +91,9 @@
 	 */
 	# Persistent:
 	var $mTagHooks, $mTransparentTagHooks, $mFunctionHooks, $mFunctionSynonyms, $mVariables,
-		$mImageParams, $mImageParamsMagicArray, $mStripList, $mMarkerIndex, $mPreprocessor,
-		$mExtLinkBracketedRegex, $mUrlProtocols, $mDefaultStripList, $mVarCache, $mConf,
-		$mFunctionTagHooks;
+		$mSubsts, $mImageParams, $mImageParamsMagicArray, $mStripList, $mMarkerIndex,
+		$mPreprocessor, $mExtLinkBracketedRegex, $mUrlProtocols, $mDefaultStripList,
+		$mVarCache, $mConf, $mFunctionTagHooks;
 
 
 	# Cleared with clearState():
@@ -2617,15 +2617,17 @@
 	}
 
 	/**
-	 * initialise the magic variables (like CURRENTMONTHNAME)
+	 * initialise the magic variables (like CURRENTMONTHNAME) and substitution modifiers 
 	 *
 	 * @private
 	 */
 	function initialiseVariables() {
 		wfProfileIn( __METHOD__ );
 		$variableIDs = MagicWord::getVariableIDs();
+		$substIDs = MagicWord::getSubstIDs();
 
 		$this->mVariables = new MagicWordArray( $variableIDs );
+		$this->mSubsts = new MagicWordArray( $substIDs );
 		wfProfileOut( __METHOD__ );
 	}
 
@@ -2796,12 +2798,19 @@
 		# SUBST
 		wfProfileIn( __METHOD__.'-modifiers' );
 		if ( !$found ) {
-			$mwSubst = MagicWord::get( 'subst' );
-			if ( $mwSubst->matchStartAndRemove( $part1 ) xor $this->ot['wiki'] ) {
-				# One of two possibilities is true:
-				# 1) Found SUBST but not in the PST phase
-				# 2) Didn't find SUBST and in the PST phase
-				# In either case, return without further processing
+
+			$substMatch = $this->mSubsts->matchVariableStartToEnd( $part1 );
+
+			# Possibilities for substMatch[0]: "subst", "safesubst" or FALSE
+			# Whether to include depends also on whether we are in the pre-save-transform
+			#
+			# safesubst || (subst && PST) => transclude (handled by if)
+			# (false && PST) || (subst && !PST)  => return input (handled by else if)
+			# false && !PST => transclude (no handling needed here)
+			if ( $substMatch[0] && ( $this->ot['wiki'] || $substMatch[0] == 'safesubst' ) ) {
+				$part1 = $substMatch[1];
+
+			} else if ( $substMatch[0] xor $this->ot['wiki'] ) {
 				$text = $frame->virtualBracketedImplode( '{{', '|', '}}', $titleWithSpaces, $args );
 				$isLocalObj = true;
 				$found = true;
Index: trunk/phase3/includes/MagicWord.php
===================================================================
--- trunk/phase3/includes/MagicWord.php	(revision 61709)
+++ trunk/phase3/includes/MagicWord.php	(revision 61710)
@@ -165,6 +165,10 @@
 		'nocontentconvert',
 	);
 
+	static public $mSubstIDs = array(
+		'subst',
+		'safesubst',
+	);
 
 	static public $mObjects = array();
 	static public $mDoubleUnderscoreArray = null;
@@ -216,6 +220,13 @@
 		return self::$mVariableIDs;
 	}
 
+	/**
+	 * Get an array of parser substitution modifier IDs
+	 */
+	static function getSubstIDs() {
+		return self::$mSubstIDs; 
+	}
+
 	/* Allow external reads of TTL array */
 	static function getCacheTTL($id) {
 		if (array_key_exists($id,self::$mCacheTTLs)) {
Index: trunk/phase3/languages/messages/MessagesEn.php
===================================================================
--- trunk/phase3/languages/messages/MessagesEn.php	(revision 61709)
+++ trunk/phase3/languages/messages/MessagesEn.php	(revision 61710)
@@ -264,7 +264,8 @@
 	'subjectpagename'        => array( 1,    'SUBJECTPAGENAME', 'ARTICLEPAGENAME' ),
 	'subjectpagenamee'       => array( 1,    'SUBJECTPAGENAMEE', 'ARTICLEPAGENAMEE' ),
 	'msg'                    => array( 0,    'MSG:'                   ),
-	'subst'                  => array( 0,    'SUBST:'                 ),
+	'subst'                  => array( 0,    'SUBST:$1'               ),
+	'safesubst'              => array( 0,    'SAFESUBST:$1'           ),
 	'msgnw'                  => array( 0,    'MSGNW:'                 ),
 	'img_thumbnail'          => array( 1,    'thumbnail', 'thumb'     ),
 	'img_manualthumb'        => array( 1,    'thumbnail=$1', 'thumb=$1'),
Index: trunk/phase3/RELEASE-NOTES
===================================================================
--- trunk/phase3/RELEASE-NOTES	(revision 61709)
+++ trunk/phase3/RELEASE-NOTES	(revision 61710)
@@ -809,6 +809,7 @@
 * (bug 22248) Output extension URLs in meta=siteinfo&siprop=extensions
 * Support key-params arrays in 'descriptionmsg' in meta=siteinfo&siprop=extensions
 * (bug 21922) YAML output should quote asterisk when used as key
+* (bug 22297) safesubst: to allow substitution without breaking transclusion
 
 === Languages updated in 1.16 ===
 

Follow-up revisions

Rev.Commit summaryAuthorDate
r61713Fix for r61710. Changing subst: to subst:$1 would cause huge problems with lo...conrad12:46, 30 January 2010
r61858Parser tests for behaviour of subst: and safesubst: after r61710conrad15:24, 2 February 2010
r62069Allow pipe trick to work after PST....conrad15:00, 6 February 2010
r62505clean r61713 (and r61710) per code reviewconrad09:34, 15 February 2010

Comments

#Comment by Tim Starling (Talk | contribs)   03:13, 15 February 2010

Review is at r61713.

Status & tagging log

Personal tools
Namespaces
Variants
Views
Actions
Site
Support
Download
Development
Communication
Toolbox