r69730 MediaWiki - Code Review archive

 			if ( strlen( $url1 ) > 255 ) {
-				return wfMsg( 'Interwiki-transclusion-url-too-long' );
+				return false;
 			}

Now that you're no longer using the transcache table with a varchar(255) field for the URL, you no longer need to do this.

+			$fullTitle = $title->getNsText().':'.$title->getText();

There's $title->getPrefixedText() for this, which also doesn't have the flaw of putting in the : for titles in the main namespace.

 			$url2 = $transAPI."?action=parse&text={{".$fullTitle."}}&prop=templates&format=json";

That's a rather nasty way to get templates, because it forces the parser to parse the template on the remote wiki. You should use ?action=query&prop=templates instead because that uses the templatelinks table. There's Title::getPrefixedText() for this, which also doesn't have the flaw of returning an extra : for pages in the main namespace.

 			$url1 = $transAPI."?action=query&prop=revisions&titles=$fullTitle&rvprop=content&format=json";

It would be good style (and prevent escaping nightmares) to build the query string with wfArrayToCGI().

 			for ($i = 0 ; $i < count( $templates ) ; $i++) {

You can use foreach ( $templates as $template ) in PHP.

+				// $newText is unused, but requesting it will put the template in the cache
+				$newText = self::fetchTemplateHTTPMaybeFromCache( $url );

So this requests each template separately. It would be nicer to request them in batches of 50 with &titles=Foo|Bar|Baz syntax. That'd need a little refactoring here and there though (of fetchTemplateHTTPMaybeFromCache, for instance). Batched fetching is the only way prefetching is gonna help at all: otherwise you might as well delay this until the moment the parser finds out it needs a template.

+		$this->mWikiID = $wikiID;

Please declare private $mWikiID; as a member explicitly near the top of the class.

> There's $title->getPrefixedText() for this, which also doesn't have the flaw of putting in the : for titles in the main namespace.

$title->getPrefixedText() will also include the interwiki prefix, which I don't want...

> Please declare private $mWikiID; as a member explicitly near the top of the class.

Ok to do this, but I can't find the list of the members of this class... They seem to be declared only in the constructor, Revision(). Weird.

Right, $title is interwiki. In that case getText() will include the namespace prefix and getNsText() willl be empty.

Oh, so they're not declared. No problem then, as long as your field is declared in the constructor.

For $title->getNsText().':'.$title->getText():

• {{mediawikiwiki::User:Peter17}} gives ":User:Peter17" (correct, except the initial ':')
• {{wikipedia:Paris-geo-stub}} gives "Template:Paris-geo-stub" (correct)

For $title->getText():

• {{mediawikiwiki::User:Peter17}} gives "User:Peter17" (correct)
• {{wikipedia:Paris-geo-stub}} gives "Paris-geo-stub" (incorrect)

Meh. Title doesn't seem to have an appropriate method for this, so I guess the best way to handle this is something like ( $title->getNamespace() != NS_MAIN ? '' : $title->getNsText() . ':' ) . $title->getText(). Probably worth creating a Title method for, something like getPrefixedTextWithoutInterwiki() but with a less horrible name.