Bryan Tong Minh wrote:
On Fri, Oct 3, 2008 at 6:29 PM, Brion Vibber brion@wikimedia.org wrote:
Perhaps it would be better to fix the setHeaders() issue, so it's called prior to execute()?
Perhaps, but that would also require the hooks to move.
Please, consider extensions that don't expect any of setHeaders(), outputHeaders() or hooks to be executed. I have one such extension, KML Export, which doesn't use $wgOut (except for some headers), and also doesn't make any sense to call the special page hooks, since it outputs KML data, not HTML.
For such special page, it makes sense to overload execute and have full control of the execution since the beginning.
It makes sense to me to have another "shallower" method to overload in order to output common (most) special pages. But I would do it a little different in a few aspects:
1. The words "execute" and "run" are almost synonyms to me, confusing. I would call it "page", "output", "render" or something else;
2. Instead of checking for is_callable(array($this, 'run')), I would create a page() method (or run(), render(), etc), and move the call_user_func call in there. That would be the "correct" way of using virtual functions.
That way, extensions can choose to overload either execute() (deeper) or page() (swallower) depending on their needs. Most special pages would overload the latter, and left execute() doing the checks it currently does.
More generally, just slapping in a replacement for the entire execute() method sounds like a bad idea. If you're extending a base class, you probably want to *alter* its behavior, not replace it. The right way to do this is to factor the code sensibly in the first place, so you only have to replace the relevant parts instead of reimplenting the whole execute() method.
I think that my proposal above fits this requirement, without breaking any existing extensions, and fulfilling Bryan's original intent.