jenkins-bot has submitted this change and it was merged.

Change subject: Rewrite to avoid messing with global state
......................................................................


Rewrite to avoid messing with global state

The main goal of this rewrite is to not use or mess with any global
state.

The ParserGetVariableValueTs and ParserGetVariableValueVarCache hooks
were replaced with setting the timestamp via
ParserOptions::setTimestamp(). This also means that {{#time:...}} from
ParserFunctions extension will correctly use the page's timestamp
instead of the current one.

Disabling tidy is also done using ParserOptions instead of changing
global state.

Change-Id: Ib2810aa5891c57831380a1a4718656cc09932b96
---
M SpecialCiteThisPage.php
M extension.json
M i18n/en.json
M i18n/qqq.json
4 files changed, 94 insertions(+), 87 deletions(-)

Approvals:
  Subramanya Sastry: Looks good to me, approved
  Jforrester: Looks good to me, but someone else must approve
  jenkins-bot: Verified



diff --git a/SpecialCiteThisPage.php b/SpecialCiteThisPage.php
index 5e1cae9..03d5fce 100644
--- a/SpecialCiteThisPage.php
+++ b/SpecialCiteThisPage.php
@@ -1,18 +1,17 @@
 <?php
 
 class SpecialCiteThisPage extends SpecialPage {
+
+       /**
+        * @var Parser
+        */
+       private $citationParser;
+
        public function __construct() {
                parent::__construct( 'CiteThisPage' );
        }
 
        public function execute( $par ) {
-               global $wgUseTidy;
-
-               // Having tidy on causes whitespace and <pre> tags to
-               // be generated around the output of the CiteThisPageOutput
-               // class TODO FIXME.
-               $wgUseTidy = false;
-
                $this->setHeaders();
                $this->outputHeader();
 
@@ -23,8 +22,7 @@
 
                if ( $title && $title->exists() ) {
                        $id = $this->getRequest()->getInt( 'id' );
-                       $cout = new CiteThisPageOutput( $title, $id );
-                       $cout->execute();
+                       $this->showCitations( $title, $id );
                }
        }
 
@@ -84,54 +82,62 @@
        protected function getGroupName() {
                return 'pagetools';
        }
-}
 
-class CiteThisPageOutput {
-       /**
-        * @var Title
-        */
-       public $mTitle;
+       private function showCitations( Title $title, $revId ) {
+               if ( !$revId ) {
+                       $revId = $title->getLatestRevID();
+               }
 
-       /**
-        * @var Article
-        */
-       public $mArticle;
+               $out = $this->getOutput();
 
-       public $mId;
+               $revision = Revision::newFromTitle( $title, $revId );
+               if ( !$revision ) {
+                       $out->wrapWikiMsg( '<div class="errorbox">$1</div>',
+                               [ 'citethispage-badrevision', 
$title->getPrefixedText(), $revId ] );
+                       return;
+               }
 
-       /**
-        * @var Parser
-        */
-       public $mParser;
+               $parserOptions = $this->getParserOptions();
+               // Set the overall timestamp to the revision's timestamp
+               $parserOptions->setTimestamp( $revision->getTimestamp() );
 
-       /**
-        * @var ParserOptions
-        */
-       public $mParserOptions;
+               $parser = $this->getParser();
+               // Register our <citation> tag which just parses using a 
different
+               // context
+               $parser->setHook( 'citation', [ $this, 'citationTag' ] );
+               // Also hold on to a separate Parser instance for <citation> 
tag parsing
+               // since we can't parse in a parse using the same Parser
+               $this->citationParser = $this->getParser();
 
-       public $mSpTitle;
+               $ret = $parser->parse(
+                       $this->getContentText(),
+                       $title,
+                       $parserOptions,
+                       /* $linestart = */ false,
+                       /* $clearstate = */ true,
+                       $revId
+               );
 
-       function __construct( $title, $id ) {
-               global $wgHooks, $wgParser;
+               $this->getOutput()->addModuleStyles( 'ext.citeThisPage' );
+               $this->getOutput()->addParserOutputContent( $ret );
 
-               $this->mTitle = $title;
-               $this->mArticle = new Article( $title );
-               $this->mId = $id;
-
-               $wgHooks['ParserGetVariableValueVarCache'][] = [ $this, 
'varCache' ];
-
-               $this->genParserOptions();
-               $this->genParser();
-
-               $wgParser->setHook( 'citation', [ $this, 'citationTagParse' ] );
        }
 
-       function execute() {
-               global $wgOut, $wgParser, $wgHooks;
+       /**
+        * @return Parser
+        */
+       private function getParser() {
+               $parserConf = $this->getConfig()->get( 'ParserConf' );
+               return new $parserConf['class']( $parserConf );
+       }
 
-               $wgHooks['ParserGetVariableValueTs'][] = [ $this, 'timestamp' ];
-
-               $msg = wfMessage( 'citethispage-content' 
)->inContentLanguage()->plain();
+       /**
+        * Get the content to parse
+        *
+        * @return string
+        */
+       private function getContentText() {
+               $msg = $this->msg( 'citethispage-content' 
)->inContentLanguage()->plain();
                if ( $msg == '' ) {
                        # With MediaWiki 1.20 the plain text files were deleted
                        # and the text moved into SpecialCite.i18n.php
@@ -146,49 +152,49 @@
                                $msg = file_get_contents( 
"${dir}citethispage-content" );
                        }
                }
-               $ret = $wgParser->parse(
-                       $msg, $this->mTitle, $this->mParserOptions, false, 
true, $this->getRevId()
+
+               return $msg;
+       }
+
+       /**
+        * Get the common ParserOptions for both parses
+        *
+        * @return ParserOptions
+        */
+       private function getParserOptions() {
+               $parserOptions = ParserOptions::newFromUser( $this->getUser() );
+               $parserOptions->setDateFormat( 'default' );
+               $parserOptions->setEditSection( false );
+
+               // Having tidy on causes whitespace and <pre> tags to
+               // be generated around the output of the CiteThisPageOutput
+               // class TODO FIXME.
+               $parserOptions->setTidy( false );
+
+               return $parserOptions;
+       }
+
+       /**
+        * Implements the <citation> tag.
+        *
+        * This is a hack to allow content that is typically parsed
+        * using the page's timestamp/pagetitle to use the current
+        * request's time and title
+        *
+        * @param string $text
+        * @param array $params
+        * @param Parser $parser
+        * @return string
+        */
+       public function citationTag( $text, $params, Parser $parser ) {
+               $ret = $this->citationParser->parse(
+                       $text,
+                       $this->getPageTitle(),
+                       $this->getParserOptions(),
+                       /* $linestart = */ false
                );
-               $wgOut->addModuleStyles( 'ext.citeThisPage' );
-
-               $wgOut->addParserOutputContent( $ret );
-       }
-
-       function genParserOptions() {
-               global $wgUser;
-               $this->mParserOptions = ParserOptions::newFromUser( $wgUser );
-               $this->mParserOptions->setDateFormat( 'default' );
-               $this->mParserOptions->setEditSection( false );
-       }
-
-       function genParser() {
-               $this->mParser = new Parser;
-               $this->mSpTitle = SpecialPage::getTitleFor( 'CiteThisPage' );
-       }
-
-       function citationTagParse( $in, $argv ) {
-               $ret = $this->mParser->parse( $in, $this->mSpTitle, 
$this->mParserOptions, false );
 
                return $ret->getText();
-       }
 
-       function varCache() {
-               return false;
-       }
-
-       function timestamp( &$parser, &$ts ) {
-               if ( isset( $parser->mTagHooks['citation'] ) ) {
-                       $ts = wfTimestamp( TS_UNIX, 
$this->mArticle->getTimestamp() );
-               }
-
-               return true;
-       }
-
-       function getRevId() {
-               if ( $this->mId ) {
-                       return $this->mId;
-               } else {
-                       return $this->mTitle->getLatestRevID();
-               }
        }
 }
diff --git a/extension.json b/extension.json
index 3a423ab..e02d812 100644
--- a/extension.json
+++ b/extension.json
@@ -37,7 +37,6 @@
        },
        "AutoloadClasses": {
                "SpecialCiteThisPage": "SpecialCiteThisPage.php",
-               "CiteThisPageOutput": "SpecialCiteThisPage.php",
                "CiteThisPageHooks": "CiteThisPage.hooks.php"
        },
        "manifest_version": 1
diff --git a/i18n/en.json b/i18n/en.json
index 56deb4e..8313337 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -10,5 +10,6 @@
        "citethispage-summary": "",
        "citethispage-change-submit": "Cite",
        "citethispage-change-target": "Page:",
+       "citethispage-badrevision": "Error: could not find any revision for the 
page \"$1\" with the revision ID $2.",
        "citethispage-content": "__NOTOC__\n<div 
class=\"mw-specialCiteThisPage-bibliographic\">\n\n== Bibliographic details for 
{{FULLPAGENAME}} ==\n\n* Page name: {{FULLPAGENAME}}\n* Author: {{SITENAME}} 
contributors\n* Publisher: ''{{SITENAME}}, {{int:sitesubtitle}}''.\n* Date of 
last revision: {{CURRENTDAY}} {{CURRENTMONTHNAME}} {{CURRENTYEAR}} 
{{CURRENTTIME}} UTC\n* Date retrieved: <citation>{{CURRENTDAY}} 
{{CURRENTMONTHNAME}} {{CURRENTYEAR}} {{CURRENTTIME}} UTC</citation>\n* 
Permanent URL: {{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}\n* Page 
Version ID: {{REVISIONID}}\n\n</div>\n<div class=\"plainlinks 
mw-specialCiteThisPage-styles\">\n\n== Citation styles for {{FULLPAGENAME}} 
==\n\n=== [[APA style]] ===\n{{FULLPAGENAME}}. ({{CURRENTYEAR}}, 
{{CURRENTMONTHNAME}} {{CURRENTDAY}}). ''{{SITENAME}}, {{int:sitesubtitle}}''. 
Retrieved <citation>{{CURRENTTIME}}, {{CURRENTMONTHNAME}} {{CURRENTDAY}}, 
{{CURRENTYEAR}}</citation> from 
{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}.\n\n=== [[The MLA style 
manual|MLA style]] ===\n\"{{FULLPAGENAME}}.\" ''{{SITENAME}}, 
{{int:sitesubtitle}}''. {{CURRENTDAY}} {{CURRENTMONTHABBREV}} {{CURRENTYEAR}}, 
{{CURRENTTIME}} UTC. <citation>{{CURRENTDAY}} {{CURRENTMONTHABBREV}} 
{{CURRENTYEAR}}, {{CURRENTTIME}}</citation> 
&lt;{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}&gt;.\n\n=== [[MHRA 
Style Guide|MHRA style]] ===\n{{SITENAME}} contributors, '{{FULLPAGENAME}}', 
''{{SITENAME}}, {{int:sitesubtitle}},'' {{CURRENTDAY}} {{CURRENTMONTHNAME}} 
{{CURRENTYEAR}}, {{CURRENTTIME}} UTC, 
&lt;{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}&gt; [accessed 
<citation>{{CURRENTDAY}} {{CURRENTMONTHNAME}} 
{{CURRENTYEAR}}</citation>]\n\n=== [[The Chicago Manual of Style|Chicago 
style]] ===\n{{SITENAME}} contributors, \"{{FULLPAGENAME}},\" ''{{SITENAME}}, 
{{int:sitesubtitle}},'' {{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}} 
(accessed <citation>{{CURRENTMONTHNAME}} {{CURRENTDAY}}, 
{{CURRENTYEAR}}</citation>).\n\n=== [[Council of Science Editors|CBE/CSE 
style]] ===\n{{SITENAME}} contributors. {{FULLPAGENAME}} [Internet]. 
{{SITENAME}}, {{int:sitesubtitle}}; {{CURRENTYEAR}} {{CURRENTMONTHABBREV}} 
{{CURRENTDAY}}, {{CURRENTTIME}} UTC [cited <citation>{{CURRENTYEAR}} 
{{CURRENTMONTHABBREV}} {{CURRENTDAY}}</citation>]. Available 
from:\n{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}.\n\n=== 
[[Bluebook|Bluebook style]] ===\n{{FULLPAGENAME}}, 
{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}} (last visited 
<citation>{{CURRENTMONTHNAME}} {{CURRENTDAY}}, 
{{CURRENTYEAR}}</citation>).\n\n=== [[BibTeX]] entry ===\n\n  @misc{ 
wiki:xxx,\n    author = \"{{SITENAME}}\",\n    title = \"{{FULLPAGENAME}} --- 
{{SITENAME}}{,} {{int:sitesubtitle}}\",\n    year = \"{{CURRENTYEAR}}\",\n    
url = \"{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}\",\n    note = 
\"[Online; accessed 
<citation>{{CURRENTDAY}}-{{CURRENTMONTHNAME}}-{{CURRENTYEAR}}</citation>]\"\n  
}\n\nWhen using the [[LaTeX]] package url (<code>\\usepackage{url}</code> 
somewhere in the preamble) which tends to give much more nicely formatted web 
addresses, the following may be preferred:\n\n  @misc{ wiki:xxx,\n    author = 
\"{{SITENAME}}\",\n    title = \"{{FULLPAGENAME}} --- {{SITENAME}}{,} 
{{int:sitesubtitle}}\",\n    year = \"{{CURRENTYEAR}}\",\n    url = 
\"'''\\url{'''{{canonicalurl:{{FULLPAGENAME}}|oldid={{REVISIONID}}}}'''}'''\",\n
    note = \"[Online; accessed 
<citation>{{CURRENTDAY}}-{{CURRENTMONTHNAME}}-{{CURRENTYEAR}}</citation>]\"\n  
}\n\n\n</div> <!--closing div for \"plainlinks\"-->"
 }
diff --git a/i18n/qqq.json b/i18n/qqq.json
index e374d16..1606f08 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -18,5 +18,6 @@
        "citethispage-summary": "{{notranslate}}\n\nA description message shown 
beneath the title of the Special page to explain what the point of the page is; 
generally un-used.",
        "citethispage-change-submit": "A button for users to change the page 
for which they will see a cite. See also 
{{msg-mw|citethispage-change-target}}.\n\n{{Identical|Cite}}",
        "citethispage-change-target": "A prompt for users to change the page 
for which they wish to see a cite. See also 
{{msg-mw|citethispage-change-submit}}.\n\n{{Identical|Page}}",
+       "citethispage-badrevision": "Error message if the title and/or revision 
ID cannot be found in the database",
        "citethispage-content": "Refers to {{msg-mw|Sitesubtitle}}.\n\n* This 
message is the entire text for the page Special:Cite\n* Any wikilinks in this 
message point to pages on the local wiki, so they must be translated.\n* Do not 
translate magic words like CURRENTYEAR, SITENAME etc.\n* Do not translate the 
parameter names (author, title etc.) for BibTeX entries.\n* Do not translate 
the div class plainlinks mw-specialCiteThisPage-styles."
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/311627
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib2810aa5891c57831380a1a4718656cc09932b96
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/CiteThisPage
Gerrit-Branch: master
Gerrit-Owner: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Alex Monk <a...@wikimedia.org>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: Subramanya Sastry <ssas...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to