Changed the calling protocol for function wfRunHooks() in Hooks.php.
authorEvan Prodromou <evanprodromou@users.mediawiki.org>
Sun, 13 Mar 2005 15:29:43 +0000 (15:29 +0000)
committerEvan Prodromou <evanprodromou@users.mediawiki.org>
Sun, 13 Mar 2005 15:29:43 +0000 (15:29 +0000)
Previously, this function used variable arguments to allow
different hooks to pass different parameters. However, var args
silently convert reference-calling to value-calling. So a call
that used to work like this:

    # old
    wfRunHooks('SomeEvent', $param1, &$param2, $param3);

...now works like this:

    # new
    wfRunHooks('SomeEvent', array($param1, &$param2, $param3));

Hook functions can now change pass-by-reference parameters correctly
(e.g. $param2 in the above example).

All calls to wfRunHooks() were changed and tested, and the change
was documented in docs/hooks.doc. This change was originally checked
in on REL1_4 branch as a bugfix, but per vibber reverted and checked
in to HEAD instead.

docs/hooks.doc
includes/Article.php
includes/EditPage.php
includes/Hooks.php
includes/SpecialBlockip.php
includes/SpecialEmailuser.php
includes/SpecialUserlogin.php
includes/SpecialUserlogout.php
includes/Title.php

index 6f5120e..78b66ba 100644 (file)
@@ -33,7 +33,7 @@ write extensions. Hooks are a principled alternative to local patches.
 Consider, for example, two options in MediaWiki. One reverses the
 order of a title before displaying the article; the other converts the
 title to all uppercase letters. Currently, in MediaWiki code, we
-handle this as follows:
+would handle this as follows (note: not real code, here):
 
     function showAnArticle($article) {
         global $wgReverseTitle, $wgCapitalizeTitle;
@@ -76,9 +76,12 @@ option-specific stuff in our mainline code. Using hooks, the function
 becomes:
 
     function showAnArticle($article) {
-       if (wfRunHooks('ArticleShow', $article)) {
+
+       if (wfRunHooks('ArticleShow', array(&$article))) {
+       
            # code to actually show the article goes here
-           wfRunHooks('ArticleShowComplete', $article);
+           
+           wfRunHooks('ArticleShowComplete', array(&$article));
        }
     }
 
@@ -89,7 +92,7 @@ on, and make changes or fix bugs.
 
 In addition, we can take all the code that deals with the little-used
 title-reversing options (say) and put it in one place. Instead of
-having a little title-reversing if-block spread all over the codebase
+having little title-reversing if-blocks spread all over the codebase
 in showAnArticle, deleteAnArticle, exportArticle, etc., we can
 concentrate it all in an extension file:
 
@@ -117,8 +120,9 @@ place means that it's easier to read and understand; you don't have to
 do a grep-find to see where the $wgReverseTitle variable is used, say.
 
 If the code is well enough isolated, it can even be excluded when not
-used -- making for some slight savings in memory and time at runtime.
-Admins who want to have all the reversed titles can add:
+used -- making for some slight savings in memory and load-up
+performance at runtime. Admins who want to have all the reversed
+titles can add:
 
     require_once('extensions/ReverseTitle.php');
     
@@ -179,6 +183,7 @@ This code would result in ircNotify being run twice when an article is
 saved: once for 'TimStarling', and once for 'brion'.
 
 Hooks can return three possible values:
+
   * true: the hook has operated successfully
   * "some string": an error occurred; processing should
          stop and the error should be shown to the user
@@ -198,7 +203,7 @@ could do:
     }
 
 Returning false makes less sense for events where the action is
-complete, and will probably be ignored.
+complete, and will normally be ignored.
 
 ==Using hooks==
 
@@ -209,9 +214,9 @@ the hooks related to a particular event, like so:
         # ...
        function protect() {
            global $wgUser;
-           if (wfRunHooks('ArticleProtect', $this, $wgUser)) {
+           if (wfRunHooks('ArticleProtect', array(&$this, &$wgUser))) {
                # protect the article
-               wfRunHooks('ArticleProtectComplete', $this, $wgUser);
+               wfRunHooks('ArticleProtectComplete', array(&$this, &$wgUser));
            }
        }
                                                    
@@ -221,6 +226,12 @@ if it shouldn't (an error occurred, or one of the hooks handled the
 action already). Checking the return value matters more for "before"
 hooks than for "complete" hooks.
 
+Note that hook parameters are passed in an array; this is a necessary
+inconvenience to make it possible to pass reference values (that can
+be changed) into the hook code. Also note that earlier versions of
+wfRunHooks took a variable number of arguments; the array() calling
+protocol came about after MediaWiki 1.4rc1.
+
 ==Events and parameters==
 
 This is a list of known events and parameters; please add to it if
@@ -288,6 +299,13 @@ $from: address of sending user
 $subject: subject of the mail
 $text: text of the mail
 
+'TitleMoveComplete': after moving an article (title)
+$old: old title
+$nt: new title
+$user: user who did the move
+$oldid: old article database ID
+$newid: new article database ID
+
 'UnknownAction': An unknown "action" has occured (useful for defining
                 your own actions)
 $action: action name
index 16bee21..4e56f69 100644 (file)
@@ -1194,13 +1194,13 @@ class Article {
                        return;
                }
 
-               if (wfRunHooks('WatchArticle', $wgUser, $this)) {
+               if (wfRunHooks('WatchArticle', array(&$wgUser, &$this))) {
                        
                        $wgUser->addWatch( $this->mTitle );
                        $wgUser->saveSettings();
 
-                       wfRunHooks('WatchArticleComplete', $wgUser, $this);
-                       
+                       wfRunHooks('WatchArticleComplete', array(&$wgUser, &$this));
+
                        $wgOut->setPagetitle( wfMsg( 'addedwatch' ) );
                        $wgOut->setRobotpolicy( 'noindex,follow' );
                        
@@ -1229,12 +1229,12 @@ class Article {
                        return;
                }
 
-               if (wfRunHooks('UnwatchArticle', $wgUser, $this)) {
+               if (wfRunHooks('UnwatchArticle', array(&$wgUser, &$this))) {
                        
                        $wgUser->removeWatch( $this->mTitle );
                        $wgUser->saveSettings();
                        
-                       wfRunHooks('UnwatchArticleComplete', $wgUser, $this);
+                       wfRunHooks('UnwatchArticleComplete', array(&$wgUser, &$this));
                        
                        $wgOut->setPagetitle( wfMsg( 'removedwatch' ) );
                        $wgOut->setRobotpolicy( 'noindex,follow' );
@@ -1287,7 +1287,7 @@ class Article {
                        if( !$moveonly ) {
                                $restrictions .= ":edit=" . $limit;
                        }
-                       if (wfRunHooks('ArticleProtect', $this, $wgUser, $limit == 'sysop', $reason, $moveonly)) {
+                       if (wfRunHooks('ArticleProtect', array(&$this, &$wgUser, $limit == 'sysop', $reason, $moveonly))) {
                                
                                $dbw =& wfGetDB( DB_MASTER );
                                $dbw->update( 'page',
@@ -1299,7 +1299,7 @@ class Article {
                                                                                           ), 'Article::protect'
                                                          );
                                
-                               wfRunHooks('ArticleProtectComplete', $this, $wgUser, $limit == 'sysop', $reason, $moveonly);
+                               wfRunHooks('ArticleProtectComplete', array(&$this, &$wgUser, $limit == 'sysop', $reason, $moveonly));
                                
                                $log = new LogPage( 'protect' );
                                if ( $limit === '' ) {
@@ -1562,7 +1562,7 @@ class Article {
                $fname = 'Article::doDelete';
                wfDebug( $fname."\n" );
 
-               if (wfRunHooks('ArticleDelete', $this, $wgUser, $reason)) {
+               if (wfRunHooks('ArticleDelete', array(&$this, &$wgUser, &$reason))) {
                        if ( $this->doDeleteArticle( $reason ) ) {
                                $deleted = $this->mTitle->getPrefixedText();
                                
@@ -1578,7 +1578,7 @@ class Article {
                                
                                $wgOut->addHTML( '<p>' . $text . "</p>\n" );
                                $wgOut->returnToMain( false );
-                               wfRunHooks('ArticleDeleteComplete', $this, $wgUser, $reason);
+                               wfRunHooks('ArticleDeleteComplete', array(&$this, &$wgUser, $reason));
                        } else {
                                $wgOut->fatalError( wfMsg( 'cannotdelete' ) );
                        }
index 544e2bc..36a6d01 100644 (file)
@@ -336,13 +336,14 @@ class EditPage {
                                        $wgOut->redirect( $this->mTitle->getFullURL() );
                                        return;
                                }
-                               if (wfRunHooks('ArticleSave', $this->mArticle, $wgUser, $this->textbox1, 
-                                                          $this->summary, $this->minoredit, $this->watchthis, NULL))
+                               if (wfRunHooks('ArticleSave', array(&$this->mArticle, &$wgUser, &$this->textbox1,
+                                                          &$this->summary, &$this->minoredit, &$this->watchthis, NULL)))
                                {
                                        $this->mArticle->insertNewArticle( $this->textbox1, $this->summary,
                                                                                                           $this->minoredit, $this->watchthis );
-                                       wfRunHooks('ArticleSaveComplete', $this->mArticle, $wgUser, $this->textbox1, 
-                                                          $this->summary, $this->minoredit, $this->watchthis, NULL);
+                                       wfRunHooks('ArticleSaveComplete', array(&$this->mArticle, &$wgUser, $this->textbox1,
+                                                                                                                       $this->summary, $this->minoredit,
+                                                                                                                       $this->watchthis, NULL));
                                }
                                return;
                        }
@@ -403,15 +404,17 @@ class EditPage {
                                        }
                                }
                                
-                               if (wfRunHooks('ArticleSave', $this->mArticle, $wgUser, $text, $this->summary,
-                                                          $this->minoredit, $this->watchthis, $sectionanchor))
+                               if (wfRunHooks('ArticleSave', array(&$this->mArticle, &$wgUser, &$text,
+                                                                                                       &$this->summary, &$this->minoredit,
+                                                                                                       &$this->watchthis, &$sectionanchor)))
                                {
                                        # update the article here
                                        if($this->mArticle->updateArticle( $text, $this->summary, $this->minoredit,
                                                                                                           $this->watchthis, '', $sectionanchor ))
                                        {
-                                               wfRunHooks('ArticleSaveComplete', $this->mArticle, $wgUser, $text, $this->summary,
-                                                                  $this->minoredit, $this->watchthis, $sectionanchor);
+                                               wfRunHooks('ArticleSaveComplete', array(&$this->mArticle, &$wgUser, $text,
+                                                                                                                               $this->summary, $this->minoredit,
+                                                                                                                               $this->watchthis, $sectionanchor));
                                                return;
                                        }
                                        else
index b156a6a..9198d00 100644 (file)
@@ -1,7 +1,7 @@
 <?php
 /**
  * Hooks.php -- a tool for running hook functions
- * Copyright 2004, Evan Prodromou <evan@wikitravel.org>.
+ * Copyright 2004, 2005 Evan Prodromou <evan@wikitravel.org>.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
 
 if (defined('MEDIAWIKI')) {
        
-/** 
- * Because programmers assign to $wgHooks, we need to be very
- * careful about its contents. So, there's a lot more error-checking
- * in here than would normally be necessary.
- */
-function wfRunHooks() {
+       /** 
+        * Because programmers assign to $wgHooks, we need to be very
+        * careful about its contents. So, there's a lot more error-checking
+        * in here than would normally be necessary.
+        */
+       
+       function wfRunHooks($event, $args) {
                
-       global $wgHooks;
-
-       if (!is_array($wgHooks)) {
-               wfDieDebugBacktrace("Global hooks array is not an array!\n");
-               return false;
-       }
-
-       $args = func_get_args();
-
-       if (count($args) < 1) {
-               wfDieDebugBacktrace("No event name given for wfRunHooks().\n");
-               return false;
-       }
-
-       $event = array_shift($args);
-
-       if (!array_key_exists($event, $wgHooks)) {
-               return true;
-       }
+               global $wgHooks;
 
-       if (!is_array($wgHooks[$event])) {
-               wfDieDebugBacktrace("Hooks array for event '$event' is not an array!\n");
-               return false;
-       }
+               if (!is_array($wgHooks)) {
+                       wfDieDebugBacktrace("Global hooks array is not an array!\n");
+                       return false;
+               }
 
-       foreach ($wgHooks[$event] as $hook) {
+               if (!array_key_exists($event, $wgHooks)) {
+                       return true;
+               }
                
-               $object = NULL;
-               $method = NULL;
-               $func = NULL;
-               $data = NULL;
-               $have_data = false;
-
-               /* $hook can be: a function, an object, an array of $function and $data,
-                * an array of just a function, an array of object and method, or an
-                * array of object, method, and data.
-                */
+               if (!is_array($wgHooks[$event])) {
+                       wfDieDebugBacktrace("Hooks array for event '$event' is not an array!\n");
+                       return false;
+               }
                
-               if (is_array($hook)) {
-                       if (count($hook) < 1) {
-                               wfDieDebugBacktrace("Empty array in hooks for " . $event . "\n");
-                       } else if (is_object($hook[0])) {
-                               $object = $hook[0];
-                               if (count($hook) < 2) {
-                                       $method = "on" . $event;
-                               } else {
-                                       $method = $hook[1];
-                                       if (count($hook) > 2) {
-                                               $data = $hook[2];
+               foreach ($wgHooks[$event] as $hook) {
+                       
+                       $object = NULL;
+                       $method = NULL;
+                       $func = NULL;
+                       $data = NULL;
+                       $have_data = false;
+                       
+                       /* $hook can be: a function, an object, an array of $function and $data,
+                        * an array of just a function, an array of object and method, or an
+                        * array of object, method, and data.
+                        */
+                       
+                       if (is_array($hook)) {
+                               if (count($hook) < 1) {
+                                       wfDieDebugBacktrace("Empty array in hooks for " . $event . "\n");
+                               } else if (is_object($hook[0])) {
+                                       $object = $hook[0];
+                                       if (count($hook) < 2) {
+                                               $method = "on" . $event;
+                                       } else {
+                                               $method = $hook[1];
+                                               if (count($hook) > 2) {
+                                                       $data = $hook[2];
+                                                       $have_data = true;
+                                               }
+                                       }
+                               } else if (is_string($hook[0])) {
+                                       $func = $hook[0];
+                                       if (count($hook) > 1) {
+                                               $data = $hook[1];
                                                $have_data = true;
                                        }
+                               } else {
+                                       wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n");
                                }
-                       } else if (is_string($hook[0])) {
-                               $func = $hook[0];
-                               if (count($hook) > 1) {
-                                       $data = $hook[1];
-                                       $have_data = true;
-                               }
+                       } else if (is_string($hook)) { # functions look like strings, too
+                               $func = $hook;
+                       } else if (is_object($hook)) {
+                               $object = $hook;
+                               $method = "on" . $event;
                        } else {
                                wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n");
                        }
-               } else if (is_string($hook)) { # functions look like strings, too
-                       $func = $hook;
-               } else if (is_object($hook)) {
-                       $object = $hook;
-                       $method = "on" . $event;
-               } else {
-                       wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n");
-               }
-
-               if ($have_data) {
-                       $hook_args = array_merge(array($data), $args);
-               } else {
-                       $hook_args = $args;
-               }
-               
-               if ($object) {
-                       $retval = call_user_func_array(array($object, $method), $hook_args);
-               } else {
-                       $retval = call_user_func_array($func, $hook_args);
+                       
+                       /* We put the first data element on, if needed. */
+                       
+                       if ($have_data) {
+                               $hook_args = array_merge(array($data), $args);
+                       } else {
+                               $hook_args = $args;
+                       }
+                       
+                       /* Call the hook. */
+                       
+                       if ($object) {
+                               $retval = call_user_func_array(array($object, $method), $hook_args);
+                       } else {
+                               $retval = call_user_func_array($func, $hook_args);
+                       }
+                       
+                       /* String return is an error; false return means stop processing. */
+                       
+                       if (is_string($retval)) {
+                               global $wgOut;
+                               $wgOut->fatalError($retval);
+                               return false;
+                       } else if (!$retval) {
+                               return false;
+                       }
                }
                
-               if (is_string($retval)) {
-                       global $wgOut;
-                       $wgOut->fatalError($retval);
-                       return false;
-               } else if (!$retval) {
-                       return false;
-               }
+               return true;
        }
-       
-       return true;
-}
 } /* if defined(MEDIAWIKI) */ 
-?>
\ No newline at end of file
+?>             
\ No newline at end of file
index 9c60423..fe34d41 100644 (file)
@@ -172,10 +172,11 @@ class IPBlockForm {
                $ban = new Block( $this->BlockAddress, $userId, $wgUser->getID(), 
                        $this->BlockReason, wfTimestampNow(), 0, $expiry );
                
-               if (wfRunHooks('BlockIp', $ban, $wgUser)) {
+               if (wfRunHooks('BlockIp', array(&$ban, &$wgUser))) {
+                       
                        $ban->insert();
                        
-                       wfRunHooks('BlockIpComplete', $ban, $wgUser);
+                       wfRunHooks('BlockIpComplete', array($ban, $wgUser));
                        
                        # Make log entry
                        $log = new LogPage( 'block' );
index 7bb4a61..577a8e9 100644 (file)
@@ -143,7 +143,7 @@ class EmailUserForm {
                $from = wfQuotedPrintable( $wgUser->getName() ) . " <" . $wgUser->getEmail() . ">";
                $subject = wfQuotedPrintable( $this->subject );
                
-               if (wfRunHooks('EmailUser', $this->mAddress, $from, $subject, $this->text)) {
+               if (wfRunHooks('EmailUser', array(&$this->mAddress, &$from, &$subject, &$this->text))) {
                        
                        $mailResult = userMailer( $this->mAddress, $from, $subject, $this->text );
                        
@@ -151,7 +151,7 @@ class EmailUserForm {
                                $titleObj = Title::makeTitle( NS_SPECIAL, "Emailuser" );
                                $encTarget = wfUrlencode( $this->target );
                                $wgOut->redirect( $titleObj->getFullURL( "target={$encTarget}&action=success" ) );
-                               wfRunHooks('EmailUserComplete', $this->mAddress, $from, $subject, $this->text);
+                               wfRunHooks('EmailUserComplete', array($this->mAddress, $from, $subject, $this->text));
                        } else {
                          $wgOut->addHTML( wfMsg( "usermailererror" ) . $mailResult);
                        }
index 28bb93c..76db17b 100644 (file)
@@ -461,7 +461,7 @@ class LoginForm {
 
                # Run any hooks; ignore results
                
-               wfRunHooks('UserLoginComplete', $wgUser);
+               wfRunHooks('UserLoginComplete', array(&$wgUser));
                
                $wgOut->setPageTitle( wfMsg( 'loginsuccesstitle' ) );
                $wgOut->setRobotpolicy( 'noindex,nofollow' );
index 6c9a888..0a34a8e 100644 (file)
 function wfSpecialUserlogout() {
        global $wgUser, $wgOut, $returnto;
 
-       if (wfRunHooks('UserLogout', $wgUser)) {
+       if (wfRunHooks('UserLogout', array(&$wgUser))) {
                
                $wgUser->logout();
-       
+
+               wfRunHooks('UserLogoutComplete', array(&$wgUser));
+               
                $wgOut->mCookies = array();
                $wgOut->setRobotpolicy( 'noindex,nofollow' );
                $wgOut->addHTML( wfMsg( 'logouttext' ) );
                $wgOut->returnToMain();
                
-               wfRunHooks('UserLogoutComplete', $wgUser);
        }
 }
 
index 0de9cd1..dcfe6b2 100644 (file)
@@ -1443,7 +1443,7 @@ class Title {
                $u = new SearchUpdate( $newid, $this->getPrefixedDBkey(), '' );
                $u->doUpdate();
 
-               wfRunHooks( 'TitleMoveComplete', $this, $nt, $wgUser, $oldid, $newid );
+               wfRunHooks( 'TitleMoveComplete', array(&$this, &$nt, &$wgUser, $oldid, $newid) );
                return true;
        }