I recently refactored SpecialWatchlist and SpecialRecentChanges to bring them closer to 2014, creating a base class ChangesListSpecialPage for dealing with pages like these. The refactoring is available as 27 patches on gerrit, 19 of them still waiting to be merged: https://gerrit.wikimedia.org/r/#/q/project:mediawiki/core+branch:master+topi...
This brings some consistency between the two special pages (whose implementations were diverging since 2006), including some new hooks which you're going to love. There are, as far as I am aware, no breaking changes in the internal APIs (yet; I'd like to remove some cruft, but I'm afraid of causing too many fatals ;) ) nor in the user interface.
This should ease some pain on Wikidata and Flow developers (all of whom do or want to do some ungodly things to watchlists), MobileFrontend developers (who will hopefully abandon the thought that a separate SpecialMobileWatchlist class is a good idea ;) ) and designers hoping to do some UI work on these specials (which would previously have to be implemented from scratch twice). A few other extensions use the related hooks, too: http://p.defau.lt/?VyhiMoZtYZsfrKsotU95Tg .
Below is a list of currently pending changes in stack order for your convenience. Reviews and merges (most of these are trivial) very much appreciated. (Note that issues in earlier patches are often corrected in latter ones, especially those marked with 'todo'.)
---
https://gerrit.wikimedia.org/r/#q,9c98cb3,n,z Rename Watchlist request parameters for consistency with RC's ones https://gerrit.wikimedia.org/r/#q,02ba62d,n,z SpecialWatchlist: Use FormOptions for parameter handling https://gerrit.wikimedia.org/r/#q,85ff221,n,z SpecialWatchlist: Don't overwrite context now that we don't have to https://gerrit.wikimedia.org/r/#q,b7093f5,n,z SpecialWatchlist: Reorder some stuff in #execute https://gerrit.wikimedia.org/r/#q,64141c4,n,z SpecialWatchlist: Synchronise some code with newer versions from RC https://gerrit.wikimedia.org/r/#q,d52909c,n,z SpecialWatchlist: JS enhancements to namespace selector (like RC) https://gerrit.wikimedia.org/r/#q,2bf87b7,n,z SpecialWatchlist: Split #execute into subfunctions like SpecialRecentChanges https://gerrit.wikimedia.org/r/#q,e42520b,n,z Always load 'mediawiki.special.changeslist' on SpecialRecentChanges and subclasses https://gerrit.wikimedia.org/r/#q,b3ed7e9,n,z Create ChangesListSpecialPage as a base class for Watchlist and RC https://gerrit.wikimedia.org/r/#q,e8a2298,n,z Changes list legend modules cleanup https://gerrit.wikimedia.org/r/#q,2f089b8,n,z ChangesListSpecialPage and subclasses: Reorder functions https://gerrit.wikimedia.org/r/#q,8b8f35d,n,z Change behavior of Special:Watchlist when user's watchlist is empty https://gerrit.wikimedia.org/r/#q,6cd99e7,n,z No longer display the number of rows shown on Special:Watchlist https://gerrit.wikimedia.org/r/#q,48da066,n,z ChangesListSpecialPage: Implement execute() https://gerrit.wikimedia.org/r/#q,f402316,n,z ChangesListSpecialPage: Implement buildMainQueryConds() https://gerrit.wikimedia.org/r/#q,c7aac1b,n,z ChangesListSpecialPage: Implement doMainQuery() https://gerrit.wikimedia.org/r/#q,8f01c9a,n,z ChangesListSpecialPage: Implement webOutput() https://gerrit.wikimedia.org/r/#q,f366add,n,z ChangesListSpecialPage: Stop mutating $opts in buildMainQueryConds() https://gerrit.wikimedia.org/r/#q,94f2465,n,z ChangesListSpecialPage: Implement two new hooks superseding 4 old ones
Hey,
Some suggestions regarding code quality and design:
* The MediaWiki SpecialPage system has serious design issues and is best treated as legacy API. I recommend not binding domain or application logic to it where this can be avoided. This means not having most code in a subclass of SpecialPage.
* Creating inheritance hierarchies to share code is very likely a bad idea. This is done often in MediaWiki, and in all cases I'm aware of has very negative consequences. Try separating concerns and using composition.
* Use hooks with caution. They can easily lead to lots of tight coupling and harmful procedural control flows. I suggest only introducing hooks that are clearly needed, and keeping argument lists of these to a minimum. This means both argument count and argument size (passing in a whole context object is likely bad for instance).
That being said, great you are working on improving this code! Also note how these are general suggestions/concerns that came to my mind after reading your email, not feedback on specific code or changes.
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
I'd agree with the general statement on inheritance (which can have weird coupling and diamonds of doom) and hooks (which can lead to hard-to-specify behavior and tangle). I'm not familiar with the main problem with SpecialPage having been articulated though.
-- View this message in context: http://wikimedia.7.x6.nabble.com/Watchlist-and-RC-refactor-and-why-you-will-... Sent from the Wikipedia Developers mailing list archive at Nabble.com.
Hi,
Jeroen's suggestions are very interesting to me. There is one thing I don't understand: Why is it a bad idea to provide a hook handler with as much context information as possible? Isn't is better to have context injected to the hook handler than to have the hook handler look for the context in some other way? Maybe even by accessing global state?
-- Robert Vogel
-----Ursprüngliche Nachricht----- Von: wikitech-l-bounces@lists.wikimedia.org [mailto:wikitech-l-bounces@lists.wikimedia.org] Im Auftrag von Aaron Schulz Gesendet: Freitag, 10. Januar 2014 04:14 An: wikitech-l@lists.wikimedia.org Betreff: Re: [Wikitech-l] Watchlist and RC refactor, and why you will like it
I'd agree with the general statement on inheritance (which can have weird coupling and diamonds of doom) and hooks (which can lead to hard-to-specify behavior and tangle). I'm not familiar with the main problem with SpecialPage having been articulated though.
-- View this message in context: http://wikimedia.7.x6.nabble.com/Watchlist-and-RC-refactor-and-why-you-will-... Sent from the Wikipedia Developers mailing list archive at Nabble.com.
_______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Thu, Jan 9, 2014 at 6:28 AM, Jeroen De Dauw jeroendedauw@gmail.comwrote:
Hey,
Some suggestions regarding code quality and design:
- The MediaWiki SpecialPage system has serious design issues and is best
treated as legacy API. I recommend not binding domain or application logic to it where this can be avoided. This means not having most code in a subclass of SpecialPage.
I agree that you shouldn't have much logic in the SpecialPage subclass, as that class is very high-level and meant for showing the output and handling of input. All *real* work should be delegated elsewhere.
I don't think it's a legacy API at all--it's certainly better than the mess of Action classes (not that we couldn't clean it up a bit, but I digress) What alternative would you propose for people to use? Implementing a content handler?
-Chad
wikitech-l@lists.wikimedia.org