On 26/04/05, Tim Starling <t.starling(a)physics.unimelb.edu.au> wrote:
On 25/04/05,
Tim Starling <timstarling(a)users.sourceforge.net> wrote:
As I said on
#mediawiki when I was whinging about bug 1770, if a
solution involves globals, it's the wrong solution. I'm not surprised
that my quick hack caused side-effects, but I really hate link table
corruption so I wanted to fix that first.
Well, as I say, in the code I'm working on (will upload to
http://bugzilla.wikimedia.org/show_bug.cgi?id=689 soon; no, really, I
finally finished what I'd started) - and I notice in your recent
change also - that hack doesn't require a global, because it is
calling a Parser function from within Parser.
But that was, of course, beside the point: the question is, should it
be possible to replace the link-holders on only part of the text, or
is this not an intended ability? The way the function was implemented
(using a global array and a string of text as an input) suggested that
it didn't operate on the article as a whole, but emptying the array of
link-holders each call violated that.
However, changes since seem to have reverted to this behaving as
expected (the link-holders are replaced in the provided string, and
others links still work).
Perhaps a better solution would be to break up
Skin::makeLinkObj() to
the point where a workalike function can be put in Parser with minimal
code duplication. The function in Parser would add link holders to a
Parser member variable, and the Skin::makeLinkObj() would just resolve
links immediately. Skin::makeLinkObj() would mainly be called for
navigation links and special pages.
As far as I understand what you're suggesting here, it doesn't really
solve the current issue, which is that within an image caption, the
links need to be parsed recursively, but not replaced with
link-holders - so that, when used in alt and/or title attributes, the
HTML tags can be stripped back out and leave the text.
The current approach does seem kind of wasteful, in that it replaces
them with link-holders and then immediately resolves them afterwards;
but in order to not replace them at all, the whole of
replaceInternalLinks() needs to run in a "resolve immediately" kind of
way. Was this what the "postParseLinkColor" system (which I notice
you've just removed) used to do, or was that something different?
In general, which approach do you think seems better?
(A)
$foo = $this->replaceInternalLinks($foo);
$foo = $this->replaceLinkHolders($foo);
(B)
$this->resolveLinksStraightAway(true); # or $skin->... or whatever
$foo = $this->replaceInternalLinks($foo);
$this->resolveLinksStraightAway(false); # or $skin->... or whatever
I guess the third option is to only use placeholders for the part of
the markup that depends on the target's existence, which is presumably
just a CSS class or whatever (thus leaving other parts of the code to
separate the HTML tags from the displayed text). The problem then
being people who want the old-style "link-name?" rather than
colour-distinctions, I guess.
--
Rowan Collins BSc
[IMSoP]