Last modified: 2011-03-13 18:05:20 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 7405 - Make Linker methods static
Make Linker methods static
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.9.x
All All
: Lowest enhancement with 1 vote (vote)
: ---
Assigned To: Dan Li
: patch, patch-need-review
Depends on:
Blocks: code_quality
  Show dependency treegraph
 
Reported: 2006-09-25 02:46 UTC by Dan Li
Modified: 2011-03-13 18:05 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
patch for phase3 (183.73 KB, patch)
2006-10-05 08:45 UTC, Dan Li
Details
patch for extensions (22.85 KB, patch)
2006-10-05 08:47 UTC, Dan Li
Details
patch (phase3) (181.85 KB, patch)
2006-10-21 05:29 UTC, Dan Li
Details
patch (extensions) (18.84 KB, patch)
2006-10-21 05:29 UTC, Dan Li
Details
updated patch (extensions) (18.34 KB, patch)
2006-11-07 02:18 UTC, Dan Li
Details
updated patch (phase3) (182.04 KB, patch)
2006-11-07 02:20 UTC, Dan Li
Details

Description Dan Li 2006-09-25 02:46:43 UTC
Methods of the Linker class can and should be static, as Linker 1) is an utility
class and 2) stores no state information.

See comments for bug 7357.
Comment 1 Dan Li 2006-10-05 08:45:36 UTC
Created attachment 2449 [details]
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.
Comment 2 Dan Li 2006-10-05 08:47:50 UTC
Created attachment 2450 [details]
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.
Comment 3 Brion Vibber 2006-10-05 18:56:52 UTC
Make sure you haven't broken those extensions on older MW versions.
Comment 4 Dan Li 2006-10-21 05:29:09 UTC
Created attachment 2528 [details]
patch (phase3)

Updated patch to latest revision.
Comment 5 Dan Li 2006-10-21 05:29:53 UTC
Created attachment 2529 [details]
patch (extensions)

Patch for extensions that preserves backwards compatibility, I believe.
Comment 6 Dan Li 2006-11-07 02:18:09 UTC
Created attachment 2650 [details]
updated patch (extensions)

Updated patch for extensions to current revision.
Comment 7 Dan Li 2006-11-07 02:20:02 UTC
Created attachment 2651 [details]
updated patch (phase3)

Updated patch for phase3. Ugh, this patch is hard to maintain.
Comment 8 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-11-07 05:53:43 UTC
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.
Comment 9 Dan Li 2006-11-07 07:17:24 UTC
(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!
Comment 10 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-11-08 05:24:19 UTC
Patches applied by Nick Jenkins in me, r17479 and r17480.
Comment 11 Nick Jenkins 2006-11-08 06:05:19 UTC
Minor update applied in r17481 to prevent parserTests regressions.
Comment 12 Tim Starling 2006-11-08 07:00:01 UTC
Breaks HTML dump, reverting. Skins are polymorphic, you need a factory.
Comment 13 Nick Jenkins 2006-11-08 10:09:38 UTC
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.
Comment 14 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-07-04 19:11:19 UTC
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?
Comment 15 Dan Li 2007-07-05 16:02:43 UTC
Yea..I thought this thing was closed a long time ago.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links