From 08d460d384c149cbfef874acb843a9ba1fc84da6 Mon Sep 17 00:00:00 2001 From: Happy-melon Date: Thu, 14 Apr 2011 12:17:24 +0000 Subject: [PATCH] Follow-up r 86041 per CR and IRC: * Article constructor needs to be called with zero as second parameter * Run stylize.php over new files * Add Action::getLang() for consistency with other context accessors * Fix declaration of FormAction::alterForm(), doesn't need to be passed by reference * Fix inline use of Credits::getCredits() in SkinTemplate and SkinLegacy --- includes/Action.php | 87 ++++++++++++++++-------------- includes/SkinLegacy.php | 2 +- includes/SkinTemplate.php | 2 +- includes/actions/CreditsAction.php | 6 +-- includes/actions/PurgeAction.php | 26 ++++----- includes/actions/WatchAction.php | 14 ++--- 6 files changed, 71 insertions(+), 66 deletions(-) diff --git a/includes/Action.php b/includes/Action.php index 609e8e0941..9f35e6a4ea 100644 --- a/includes/Action.php +++ b/includes/Action.php @@ -43,19 +43,19 @@ abstract class Action { * @param $action String * @return bool|null|string */ - private final static function getClass( $action ){ + private final static function getClass( $action ) { global $wgActions; $action = strtolower( $action ); - if( !isset( $wgActions[$action] ) ){ + if ( !isset( $wgActions[$action] ) ) { return null; } - if( $wgActions[$action] === false ){ + if ( $wgActions[$action] === false ) { return false; } - elseif( $wgActions[$action] === true ){ + elseif ( $wgActions[$action] === true ) { return ucfirst( $action ) . 'Action'; } @@ -71,9 +71,9 @@ abstract class Action { * @return Action|false|null false if the action is disabled, null * if it is not recognised */ - public final static function factory( $action, Article $page ){ + public final static function factory( $action, Article $page ) { $class = self::getClass( $action ); - if( $class ){ + if ( $class ) { $obj = new $class( $page ); return $obj; } @@ -94,8 +94,8 @@ abstract class Action { * Get the RequestContext in use here * @return RequestContext */ - protected final function getContext(){ - if( $this->context instanceof RequestContext ){ + protected final function getContext() { + if ( $this->context instanceof RequestContext ) { return $this->context; } return $this->page->getContext(); @@ -137,11 +137,20 @@ abstract class Action { return $this->getContext()->skin; } + /** + * Shortcut to get the user Language being used for this instance + * + * @return Skin + */ + protected final function getLang() { + return $this->getContext()->lang; + } + /** * Shortcut to get the Title object from the page * @return Title */ - protected final function getTitle(){ + protected final function getTitle() { return $this->page->getTitle(); } @@ -150,7 +159,7 @@ abstract class Action { * these things in the real world * @param Article $page */ - protected function __construct( Article $page ){ + protected function __construct( Article $page ) { $this->page = $page; } @@ -175,15 +184,15 @@ abstract class Action { * @throws ErrorPageError */ protected function checkCanExecute( User $user ) { - if( $this->requiresWrite() && wfReadOnly() ){ + if ( $this->requiresWrite() && wfReadOnly() ) { throw new ReadOnlyError(); } - if( $this->getRestriction() !== null && !$user->isAllowed( $this->getRestriction() ) ){ + if ( $this->getRestriction() !== null && !$user->isAllowed( $this->getRestriction() ) ) { throw new PermissionsError( $this->getRestriction() ); } - if( $this->requiresUnblock() && $user->isBlocked() ){ + if ( $this->requiresUnblock() && $user->isBlocked() ) { $block = $user->mBlock; throw new UserBlockedError( $block ); } @@ -193,7 +202,7 @@ abstract class Action { * Whether this action requires the wiki not to be locked * @return Bool */ - public function requiresWrite(){ + public function requiresWrite() { return true; } @@ -201,7 +210,7 @@ abstract class Action { * Whether this action can still be executed by a blocked user * @return Bool */ - public function requiresUnblock(){ + public function requiresUnblock() { return true; } @@ -220,10 +229,6 @@ abstract class Action { /** * Returns the name that goes in the \ page title * - * Derived classes can override this, but usually it is easier to keep the - * default behaviour. Messages can be added at run-time, see - * MessageCache.php. - * * @return String */ protected function getDescription() { @@ -259,20 +264,20 @@ abstract class FormAction extends Action { * Add pre- or post-text to the form * @return String HTML which will be sent to $form->addPreText() */ - protected function preText(){ return ''; } - protected function postText(){ return ''; } + protected function preText() { return ''; } + protected function postText() { return ''; } /** * Play with the HTMLForm if you need to more substantially - * @param &$form HTMLForm + * @param $form HTMLForm */ - protected function alterForm( HTMLForm &$form ){} + protected function alterForm( HTMLForm $form ) {} /** * Get the HTMLForm to control behaviour * @return HTMLForm|null */ - protected function getForm(){ + protected function getForm() { $this->fields = $this->getFormFields(); // Give hooks a chance to alter the form, adding extra fields or text etc @@ -315,14 +320,14 @@ abstract class FormAction extends Action { * display something new or redirect to somewhere. Some actions have more exotic * behaviour, but that's what subclassing is for :D */ - public function show(){ + public function show() { $this->setHeaders(); // This will throw exceptions if there's a problem $this->checkCanExecute( $this->getUser() ); $form = $this->getForm(); - if( $form->show() ){ + if ( $form->show() ) { $this->onSuccess(); } } @@ -334,7 +339,7 @@ abstract class FormAction extends Action { * @param bool $captureErrors * @return bool */ - public function execute( array $data = null, $captureErrors = true ){ + public function execute( array $data = null, $captureErrors = true ) { try { // Set a new context so output doesn't leak. $this->context = clone $this->page->getContext(); @@ -343,17 +348,17 @@ abstract class FormAction extends Action { $this->checkCanExecute( $this->getUser() ); $fields = array(); - foreach( $this->fields as $key => $params ){ - if( isset( $data[$key] ) ){ + foreach ( $this->fields as $key => $params ) { + if ( isset( $data[$key] ) ) { $fields[$key] = $data[$key]; - } elseif( isset( $params['default'] ) ) { + } elseif ( isset( $params['default'] ) ) { $fields[$key] = $params['default']; } else { $fields[$key] = null; } } $status = $this->onSubmit( $fields ); - if( $status === true ){ + if ( $status === true ) { // This might do permanent stuff $this->onSuccess(); return true; @@ -361,8 +366,8 @@ abstract class FormAction extends Action { return false; } } - catch ( ErrorPageError $e ){ - if( $captureErrors ){ + catch ( ErrorPageError $e ) { + if ( $captureErrors ) { return false; } else { throw $e; @@ -388,19 +393,19 @@ abstract class FormlessAction extends Action { /** * We don't want an HTMLForm */ - protected function getFormFields(){ + protected function getFormFields() { return false; } - public function onSubmit( $data ){ + public function onSubmit( $data ) { return false; } - public function onSuccess(){ + public function onSuccess() { return false; } - public function show(){ + public function show() { $this->setHeaders(); // This will throw exceptions if there's a problem @@ -416,11 +421,11 @@ abstract class FormlessAction extends Action { * @param $captureErrors Bool whether to catch exceptions and just return false * @return Bool whether execution was successful */ - public function execute( array $data = null, $captureErrors = true){ + public function execute( array $data = null, $captureErrors = true ) { try { // Set a new context so output doesn't leak. $this->context = clone $this->page->getContext(); - if( is_array( $data ) ){ + if ( is_array( $data ) ) { $this->context->setRequest( new FauxRequest( $data, false ) ); } @@ -430,8 +435,8 @@ abstract class FormlessAction extends Action { $this->onView(); return true; } - catch ( ErrorPageError $e ){ - if( $captureErrors ){ + catch ( ErrorPageError $e ) { + if ( $captureErrors ) { return false; } else { throw $e; diff --git a/includes/SkinLegacy.php b/includes/SkinLegacy.php index 201c3f08e9..e5202bbfb5 100644 --- a/includes/SkinLegacy.php +++ b/includes/SkinLegacy.php @@ -210,7 +210,7 @@ class LegacyTemplate extends BaseTemplate { } if ( $wgMaxCredits != 0 ) { - $s .= ' ' . Credits::getCredits( $article, $wgMaxCredits, $wgShowCreditsIfMax ); + $s .= ' ' . Action::factory( 'credits', $article )->getCredits( $wgMaxCredits, $wgShowCreditsIfMax ); } else { $s .= $this->data['lastmod']; } diff --git a/includes/SkinTemplate.php b/includes/SkinTemplate.php index e5cfe2f239..65577542e9 100644 --- a/includes/SkinTemplate.php +++ b/includes/SkinTemplate.php @@ -382,7 +382,7 @@ class SkinTemplate extends Skin { $this->credits = false; if( $wgMaxCredits != 0 ){ - $this->credits = Credits::getCredits( $article, $wgMaxCredits, $wgShowCreditsIfMax ); + $this->credits = Action::factory( 'credits', $article )->getCredits( $wgMaxCredits, $wgShowCreditsIfMax ); } else { $tpl->set( 'lastmod', $this->lastModified( $article ) ); } diff --git a/includes/actions/CreditsAction.php b/includes/actions/CreditsAction.php index 576834faaa..72a96833d5 100644 --- a/includes/actions/CreditsAction.php +++ b/includes/actions/CreditsAction.php @@ -25,11 +25,11 @@ class CreditsAction extends FormlessAction { - public function getName(){ + public function getName() { return 'credits'; } - public function getRestriction(){ + public function getRestriction() { return null; } @@ -57,7 +57,7 @@ class CreditsAction extends FormlessAction { * @param $showIfMax Bool: whether to contributors if there more than $cnt * @return String: html */ - protected function getCredits( $cnt, $showIfMax = true ) { + public function getCredits( $cnt, $showIfMax = true ) { wfProfileIn( __METHOD__ ); $s = ''; diff --git a/includes/actions/PurgeAction.php b/includes/actions/PurgeAction.php index e212fbf6dd..595580d920 100644 --- a/includes/actions/PurgeAction.php +++ b/includes/actions/PurgeAction.php @@ -25,19 +25,19 @@ class PurgeAction extends FormAction { - public function getName(){ + public function getName() { return 'purge'; } - public function getRestriction(){ + public function getRestriction() { return null; } - public function requiresUnblock(){ + public function requiresUnblock() { return false; } - public function getDescription(){ + public function getDescription() { return ''; } @@ -45,11 +45,11 @@ class PurgeAction extends FormAction { * Just get an empty form with a single submit button * @return array */ - protected function getFormFields(){ + protected function getFormFields() { return array(); } - public function onSubmit( $data ){ + public function onSubmit( $data ) { $this->page->doPurge(); return true; } @@ -58,36 +58,36 @@ class PurgeAction extends FormAction { * purge is slightly wierd because it can be either formed or formless depending * on user permissions */ - public function show(){ + public function show() { $this->setHeaders(); // This will throw exceptions if there's a problem $this->checkCanExecute( $this->getUser() ); - if( $this->getUser()->isAllowed( 'purge' ) ){ + if ( $this->getUser()->isAllowed( 'purge' ) ) { $this->onSubmit( array() ); $this->onSuccess(); } else { $form = $this->getForm(); - if( $form->show() ){ + if ( $form->show() ) { $this->onSuccess(); } } } - protected function alterForm( HTMLForm $form ){ + protected function alterForm( HTMLForm $form ) { $form->setSubmitText( wfMsg( 'confirm_purge_button' ) ); } - protected function preText(){ + protected function preText() { return wfMessage( 'confirm-purge-top' )->parse(); } - protected function postText(){ + protected function postText() { return wfMessage( 'confirm-purge-bottom' )->parse(); } - public function onSuccess(){ + public function onSuccess() { $this->getOutput()->redirect( $this->getTitle() ); } } diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php index 43e0b47d74..5dace86304 100644 --- a/includes/actions/WatchAction.php +++ b/includes/actions/WatchAction.php @@ -22,23 +22,23 @@ class WatchAction extends FormlessAction { - public function getName(){ + public function getName() { return 'watch'; } - public function getRestriction(){ + public function getRestriction() { return 'read'; } - public function requiresUnblock(){ + public function requiresUnblock() { return false; } - protected function getDescription(){ + protected function getDescription() { return wfMsg( 'addedwatch' ); } - protected function checkCanExecute( User $user ){ + protected function checkCanExecute( User $user ) { if ( $user->isAnon() ) { throw new ErrorPageError( 'watchnologin', 'watchnologintext' ); } @@ -62,11 +62,11 @@ class WatchAction extends FormlessAction { class UnwatchAction extends WatchAction { - public function getName(){ + public function getName() { return 'unwatch'; } - protected function getDescription(){ + protected function getDescription() { return wfMsg( 'removedwatch' ); } -- 2.20.1