r81895 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81894‎ | r81895 | r81896 >
Date:16:14, 10 February 2011
Author:btongminh
Status:ok (Comments)
Tags:
Comment:
Add a context to SpecialPage, so that subclasses don't need globals anymore. Introduces $mRequest, $mOutput and $mFullTitle, which can be used instead of $wgRequest, $wgOut and $wgTitle. Introduces msg(), which is a wrapper around wfMessage()->title().
Modified paths:
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SpecialPage.php
@@ -74,6 +74,22 @@
7575 */
7676 var $mAddedRedirectParams = array();
7777 /**
 78+ * Current request
 79+ * @var WebRequest
 80+ */
 81+ protected $mRequest;
 82+ /**
 83+ * Current output page
 84+ * @var OutputPage
 85+ */
 86+ protected $mOutput;
 87+ /**
 88+ * Full title including $par
 89+ * @var Title
 90+ */
 91+ protected $mFullTitle;
 92+
 93+ /**
7894 * List of special pages, followed by parameters.
7995 * If the only parameter is a string, that is the page name.
8096 * Otherwise, it is an array. The format is one of:
@@ -539,6 +555,9 @@
540556 wfProfileOut( __METHOD__ );
541557 return false;
542558 }
 559+
 560+ # Page exists, set the context
 561+ $page->setContext( $wgRequest, $wgOut );
543562
544563 # Check for redirect
545564 if ( !$including ) {
@@ -951,6 +970,27 @@
952971 ? $params
953972 : false;
954973 }
 974+
 975+ /**
 976+ * Sets the context this SpecialPage is executed in
 977+ *
 978+ * @param $request WebRequest
 979+ * @param $output OutputPage
 980+ */
 981+ protected function setContext( $request, $output ) {
 982+ $this->mRequest = $request;
 983+ $this->mOutput = $output;
 984+ $this->mFullTitle = $output->getTitle();
 985+ }
 986+ /**
 987+ * Wrapper around wfMessage that sets the current context. Currently this
 988+ * is only the title.
 989+ *
 990+ * @see wfMessage
 991+ */
 992+ public function msg( /* $args */ ) {
 993+ return call_user_func_array( 'wfMessage', func_get_args() )->title( $this->mFullTitle );
 994+ }
955995 }
956996
957997 /**

Comments

#Comment by 😂 (talk | contribs)   16:19, 10 February 2011

This is 2011, do we really need to prefix member variables with $m?

For a special page, isn't the title object usually ->getTitle()?

#Comment by Nikerabbit (talk | contribs)   16:21, 10 February 2011

I don't think this line works (passing func_get_args() to method directly.

return call_user_func_array( 'wfMessage', func_get_args() )->title( $this->mFullTitle );
#Comment by IAlex (talk | contribs)   16:21, 10 February 2011

Would it be also possible to have a User object in the context?

#Comment by Nikerabbit (talk | contribs)   16:25, 10 February 2011

I wonder if the msg method is really worth it. When is the title relevant when addWikiMsg cannot be used?

#Comment by Bryan (talk | contribs)   18:05, 10 February 2011

> This is 2011, do we really need to prefix member variables with $m?

Consistency, all other variables are $mPrefixed as well.

> For a special page, isn't the title object usually ->getTitle()?

No, that does not take $par into account

> I don't think this line works (passing func_get_args() to method directly.

Tested it, it works.

> I wonder if the msg method is really worth it. When is the title relevant when addWikiMsg cannot be used?

In many cases you need to construct the output as a string, and you can't just directly use addWikiMsg. Perhaps that in most of those cases you don't need a title, but when writing a special page, you should not worry about whether you need it or not.

#Comment by Nikerabbit (talk | contribs)   22:09, 11 February 2011

Just found this nice discussion by accident: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/55168#code-comments :)

#Comment by IAlex (talk | contribs)   09:33, 3 April 2011

After some investigation, I'm not really sure $mFullTitle is needed since $wgTitle is set to "$this->getTitle()" in SpecialPage::executePath(); this mean that messages obtained through wfMessage() or wfMsg*() will have their title without subpage and messages obtained through SpecialPage::msg() will have their title with subpage.

Status & tagging log