Apologies in advance if this would be more appropriate for mediawiki- l, but since I discuss a possible modification to trunk, I thought this would be the correct list. In working on my UserRightsList extension, I encountered changes in includes/SpecialUserrights.php that have me wondering if I'm missing something really basic about oo programming and how I should have made the extension. I originally wrote the Special page extension by extending class SpecialPage and writing methods that largely duplicated things in SpecialUserrights. For 0.5, I extended class UserRightsPage (which didn't exist prior to 1.12) to be able to reuse UserRightsPage::saveUserGroups( $username, $removegroup, $addgroup, $reason = ''). This works in 1.12... but no sooner did I announce it than I got a message that it didin't work in 1.13a. It turns out that in 1.13a, that UserRightsPage::saveUserGroups() no longer passes the $removegroup and $addgroup arrays, it takes them directly from $wgRequest. Since I'm looping over a list of users, I patched the extension on 0.51 to modify $wgRequest->data for each iteration over the array of users sent on execution. This strikes me as ugly, as it changes a global that could contain information expected by something further downstream (although I think it should be OK for the Userrights hook). But I'm stumped about how else I could do this. It seems that it isn't any better to have class UserRightsList extend SpecialPage instead of UserrightsPage if all it means is instead of $this- the same problem with the method params and that is actually worse, since I have to load up the code with a bunch of things like: function fetchUser( $username ) { return UserrightsPage::fetchUser( $username ); } just to get it to work. Would it be reasonable to suggest something like changing SpecialUserrights.php to: function saveUserGroups( $username, $removegroup = '', $addgroup = '', $reason = '') { ... if (is_array($removegroup) && is_array($addgroup) ){ foreach ($allgroups as $group) { ... } } ... This would add a little overhead for the is_array checks, but would ensure backward compatibility with anything else that calls the method (perhaps that would only be me), and allow classes to extend class UserrightsPage without modifying $wgRequest. The larger questions: - Should I be avoiding this approach (extending classes in includes) altogether? - If not, is there a way to tell whether the parameters/behavior are likely to be stable? - If yes, Is there a way I should be doing this to make it less likely to break in future versions of MW? - Was Userrights just an outlier because it needed refactoring and now it should be stable? Thanks, JH ===================================== Jim Hu Associate Professor Dept. of Biochemistry and Biophysics 2128 TAMU Texas A&M Univ. College Station, TX 77843-2128 979-862-4054