dto wrote:

patch for phase3

Makes all of the methods of the Linker class static and updates calls to those
methods accordingly.

In addition, I've tried to remove as many references to Skin, $skin,
->getSkin(), ->skin, and $sk (did I miss any?) as possible. These are the major
changes:

• removed $skin argument from ImageHistoryList constructor (ImagePage.php)
• removed PageHistory::mSkin (PageHistory.php)
• removed $sk argument of ucListEdit (SpecialContributions.php)
• removed $sk argument of wfSpecialSpecialpages_gen (SpecialSpecialpages.php)
• removed WhatLinksHerePage::skin (SpecialWhatlinkshere.php)
• removed CategoryViewer::getSkin() (CategoryPage.php)
• removed $skin argument from ChangesList/EnhancedChangesList/OldChangesList

constructor (ChangesList.php)

• removed $skin parameter to FetchChangesList hook (ChangesList.php)
• removed ImageGallery::getSkin() and ImageGallery::useSkin()

(ImageGallery.php)

• made LogPage::actionText, LogPage::makeParamBlob, LogPage::extractParams

static (LogPage.php)

• LogPage::actionText takes parameter $forContent instead of $skin (only

occurrences of $skin were for checking whether $skin was null) (LogPage.php)

• removed LogViewer::skin (SpecialLog.php)
• removed IndexPager::getSkin() (Pager.php)
• removed ParserOptions::mSkin, ParserOptions::getSkin(),

ParserOptions::setSkin() (Parser.php)

• removed argument $skin from QueryPage::formatResult and from all subclasses

(QueryPage.php and many special pages)

• made Skin::drawCategoryBrowser static (was already being called statically)

(Skin.php)

• removed RevisionDeleteForm::skin (SpecialRevisiondelete.php)

I checked /extensions for stuff that these changes might break. Another patch
coming for that.

attachment linkerphase3.diff ignored as obsolete

dto wrote:

patch for extensions

Suggested changes for extensions. Many of these (the ones for Semantic
MediaWiki) are just a result of QueryPage::formatResult taking one less
argument.

attachment linkerextensions.diff ignored as obsolete

Make sure you haven't broken those extensions on older MW versions.

dto wrote:

patch (phase3)

Updated patch to latest revision.

attachment linker.diff ignored as obsolete

dto wrote:

patch (extensions)

Patch for extensions that preserves backwards compatibility, I believe.

attachment linkerextensions.diff ignored as obsolete

dto wrote:

updated patch (extensions)

Updated patch for extensions to current revision.

Attached:

dto wrote:

updated patch (phase3)

Updated patch for phase3. Ugh, this patch is hard to maintain.

Attached:

ayg wrote:

1. Please don't make unrelated changes to whitespace, comments, etc. when

submitting a patch. It just makes it harder to review.

2. I don't think @static is necessary for functions marked as static in the PHP

code.

3. How much have you tested this? I'd be willing to commit it if you've poked

things and nothing's broken, after some testing by me.

dto wrote:

(In reply to comment #8)

1. Please don't make unrelated changes to whitespace, comments, etc. when

submitting a patch. It just makes it harder to review.

Sorry. I thought that since I was changing so much of Linker.php anyways, I
might as well. (But at this point, I hope it's all right if I leave them.)

2. I don't think @static is necessary for functions marked as static in the PHP

code.

OK...should I remove those from the patch? They should eventually be removed
from the current MediaWiki code as well, then.

3. How much have you tested this? I'd be willing to commit it if you've poked

things and nothing's broken, after some testing by me.

My low-activity wiki runs. Editing/viewing pages and special pages seems OK.
After all, all I've really done in the patch is refactoring.

Thanks for your interest in this enhancement!

ayg wrote:

Patches applied by Nick Jenkins in me, r17479 and r17480.

nickpj wrote:

Minor update applied in r17481 to prevent parserTests regressions.

Breaks HTML dump, reverting. Skins are polymorphic, you need a factory.

nickpj wrote:

Summarising info from TimS on IRC in regards to his concerns about this:

When the user runs "php maintenance/dumpHTML.php", it looks like the overridden
makeBrokenLinkObj() function isn't called. The result is that broken links in
the static dump will show up as red links, instead of plain text as per normal
(i.e. there should be no red links). Would prefer to not use static functions,
and would prefer to use a factory function for best flexibility. An example of
this is getSkin() in includes/User.php.

ayg wrote:

So basically the answer to this is we want Linker methods to be overridable by skins, and specifically we want them overridden transparently to the caller, so that when some random code calls makeBrokenLinkObj() or anything else it calls it from Monobook or whatever instead of Linker, without the main code knowing or caring what class it's calling. Thus you cannot call the methods with anything like Linker::makeBrokenLinkObj(), since that won't be overridden. Thus we use non-static calls. Thus this is WONTFIX. Correct?

dto wrote:

Yea..I thought this thing was closed a long time ago.