r62035 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r62034‎ | r62035 | r62036 >
Date:16:49, 5 February 2010
Author:conrad
Status:reverted (Comments)
Tags:
Comment:
bug 5210 - add getTransclusionText() to the Parser to remove the horrible (and
very broken) attempt to reimplement bits of the preprocessor in
EditPage.php.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
@@ -493,6 +493,22 @@
494494 }
495495
496496 /**
 497+ * Get the wikitext of a page as though it was transcluded.
 498+ *
 499+ * Specifically <includeonly> etc. are parsed, redirects are followed, comments
 500+ * are removed, but templates arguments and parser functions are untouched.
 501+ *
 502+ * This is not called by the parser itself, see braceSubstitution for its transclusion.
 503+ *
 504+ * @public
 505+ */
 506+ function getTransclusionText( $title ) {
 507+ list($text, $title) = $this->getTemplateDom( $title );
 508+ $flags = PPFrame::NO_ARGS | PPFrame::NO_TEMPLATES;
 509+ return $this->getPreprocessor()->newFrame()->expand( $text, $flags );
 510+ }
 511+
 512+ /**
497513 * Get a random string
498514 *
499515 * @private
Index: trunk/phase3/includes/EditPage.php
@@ -227,22 +227,13 @@
228228 * @return string The contents of the page.
229229 */
230230 protected function getPreloadedText( $preload ) {
 231+ global $wgParser;
231232 if ( !empty( $this->mPreloadText ) ) {
232233 return $this->mPreloadText;
233 - } elseif ( $preload === '' ) {
234 - return '';
235234 } else {
236235 $preloadTitle = Title::newFromText( $preload );
237236 if ( isset( $preloadTitle ) && $preloadTitle->userCanRead() ) {
238 - $rev = Revision::newFromTitle( $preloadTitle );
239 - if ( is_object( $rev ) ) {
240 - $text = $rev->getText();
241 - // TODO FIXME: AAAAAAAAAAA, this shouldn't be implementing
242 - // its own mini-parser! -ævar
243 - $text = preg_replace( '~</?includeonly>~', '', $text );
244 - return $text;
245 - } else
246 - return '';
 237+ return $wgParser->getTransclusionText( $preloadTitle );
247238 }
248239 }
249240 }
Index: trunk/phase3/RELEASE-NOTES
@@ -825,6 +825,7 @@
826826 * (bug 18758) API read of watchlist's wl_notificationtimestamp
827827 * (bug 20809) Expose EditFormPreloadText via the API
828828 * (bug 18427) Comment (edit summary) parser option for API
 829+* (bug 5210) preload parser should parse <noinclude> (as well as <includeonly>)
829830
830831 === Languages updated in 1.16 ===
831832

Follow-up revisions

RevisionCommit summaryAuthorDate
r62049style fixes for r62035conrad21:52, 5 February 2010
r62194Pretty sure that...reedy20:46, 9 February 2010
r62689Moving Conrad's recent parser work out to a branch. Reverted r62434, r62416, ...tstarling05:19, 19 February 2010
r63194Merge fixes to ?preload= from /branches/conrad/ (cf. bug 5210, r62864, r62035)conrad02:41, 3 March 2010

Comments

#Comment by Jack Phoenix (talk | contribs)   21:48, 5 February 2010
+	 * This is not called by the parser itself, see braceSubstitution for its transclusion. 
+	 *
+	 * @public
+	 */
+	function getTransclusionText( $title ) {
+		list($text, $title) = $this->getTemplateDom( $title );

AFAIK we don't use @public (or @static or @private or anything like that) in modern code; just mark the function visibility normally (public function getTransclusionText( $title ) {) and the list() call could use a bit of spacing: list( $text, $title ) = $this->getTemplateDom( $title );

#Comment by Nikerabbit (talk | contribs)   08:55, 7 February 2010

This causes the following error to appear when editing non-existing page with section=new (i.php?title=Bar&action=edit&section=new):

PHP Fatal error: Call to a member function getTemplateCallback() on a non-object in /www/w/includes/parser/Parser.php on line 3186

It's the first line of fetchTemplateAndTitle function.

#Comment by Nikerabbit (talk | contribs)   09:05, 7 February 2010

The problem seems to be that getTransclusionText is a new entry point, but it does not initialize mOptions.

#Comment by Conrad.Irwin (talk | contribs)   12:07, 7 February 2010

See http://www.mediawiki.org/wiki/Special:Code/MediaWiki/62080, I forgot to put this bug number in the commit message.

I am not entirely sure how the parser's state is handled as the request progresses, so I have simply copied the initialisation from the other "entry points". If this is wrong, I'm afraid someone else will have to pick up the pieces; or explain it to me clearly.

Interestingly I can't replicate your bug, which presumably means that the parser is being initialised before the preload happens on my mediawiki, urgh...

#Comment by Conrad.Irwin (talk | contribs)   19:48, 12 February 2010

The bug reported by Nike is fixed, so I'm changing this back to "new".

Status & tagging log