Hi,
My extension, Semantic Forms, changes the URL of certain "broken links" (AKA red links). Up until now this has been done through a patch to the MediaWiki code (specifically to the file "/includes/Linker.php"), but I want a real solution. That is the new 'BrokenLink' I'm suggesting, which would involve adding a single line to Linker.php. Around line 348, within makeBrokenLinkObj(), under the line:
$u = $nt->escapeLocalURL( $q );
there would be added added the following:
wfRunHooks( 'BrokenLink', array( $nt, $query, &$u ));
Please let me know if you have any objections to the name of the hook, its placement or anything else. If there are no responses or objections, I guess I'll just feel free to add in the code myself (I've never modified MediaWiki code before, but I suppose there's a first time for everything.)
-Yaron
On Wed, Mar 5, 2008 at 10:51 AM, Yaron Koren yaron57@gmail.com wrote:
Around line 348, within makeBrokenLinkObj(), under the line:
$u = $nt->escapeLocalURL( $q );there would be added added the following:
wfRunHooks( 'BrokenLink', array( $nt, $query, &$u ));
That's not very flexible, is it? It basically allows changing one thing, $u. Why don't you put it almost at the bottom, or almost at the top, and have it be
$ret = null; if( !wfRunHooks( 'BrokenLink', array( &$nt, &$text, &$query, &$trail, &$prefix, &$ret ) ) ) { return $ret; }
If it's at the bottom, some other parameters might be passed, to avoid having to re-process everything. The top would maybe be better.
The name seems fine. Make sure you update docs/hooks.txt, whatever you do.
Hi,
The idea of having it at the top of the function, and making it more comprehensive, makes sense; but I don't understand the specific code you suggested. My understanding is that wfRunHooks() normally returns true; in that case, if the call is placed at the top of the function, whatever value the hooks set the broken link to will get overridden by makeBrokenLinkObj() itself. Or is the idea that every function that uses this hook should return false, thus ensuring that no more than one function is called for it?
-Yaron
On Wed, Mar 5, 2008 at 11:31 AM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 10:51 AM, Yaron Koren yaron57@gmail.com wrote:
Around line 348, within makeBrokenLinkObj(), under the line:
$u = $nt->escapeLocalURL( $q );there would be added added the following:
wfRunHooks( 'BrokenLink', array( $nt, $query, &$u ));That's not very flexible, is it? It basically allows changing one thing, $u. Why don't you put it almost at the bottom, or almost at the top, and have it be
$ret = null; if( !wfRunHooks( 'BrokenLink', array( &$nt, &$text,&$query, &$trail, &$prefix, &$ret ) ) ) { return $ret; }
If it's at the bottom, some other parameters might be passed, to avoid having to re-process everything. The top would maybe be better.
The name seems fine. Make sure you update docs/hooks.txt, whatever you do.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Wed, Mar 5, 2008 at 11:49 AM, Yaron Koren yaron57@gmail.com wrote:
The idea of having it at the top of the function, and making it more comprehensive, makes sense; but I don't understand the specific code you suggested. My understanding is that wfRunHooks() normally returns true; in that case, if the call is placed at the top of the function, whatever value the hooks set the broken link to will get overridden by makeBrokenLinkObj() itself. Or is the idea that every function that uses this hook should return false, thus ensuring that no more than one function is called for it?
The idea is that functions could do two things:
1) Override the output entirely, by setting $ret and returning false.
2) Modify the function's parameters, and return true.
I'm not sure how much more flexibility is reasonably possible. It doesn't seem like it would make much sense for multiple extensions to modify the appearance of broken links in really arbitrary ways. What would you suggest? The hook could be near the bottom instead, right before the definition of $s, with lots more parameters. By that point, some of the original parameters (like $text) have been overridden, but that could be changed.
Hi,
Okay, that makes sense - there's probably no reason to allow more than one hook to override the output. The one downside of this approach that I can see is that functions that don't override the output might not get called at all, if they're later in the array than a function that does override it. I don't think that's a big deal, since I can't imagine some large amount of custom code called for every broken link, or two extensions "collaborating" on creating a new link. Unless anyone objects, I'll go with your suggestion.
-Yaron
On Wed, Mar 5, 2008 at 12:02 PM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 11:49 AM, Yaron Koren yaron57@gmail.com wrote:
The idea of having it at the top of the function, and making it more comprehensive, makes sense; but I don't understand the specific code
you
suggested. My understanding is that wfRunHooks() normally returns true;
in
that case, if the call is placed at the top of the function, whatever
value
the hooks set the broken link to will get overridden by
makeBrokenLinkObj()
itself. Or is the idea that every function that uses this hook should
return
false, thus ensuring that no more than one function is called for it?
The idea is that functions could do two things:
Override the output entirely, by setting $ret and returning false.
Modify the function's parameters, and return true.
I'm not sure how much more flexibility is reasonably possible. It doesn't seem like it would make much sense for multiple extensions to modify the appearance of broken links in really arbitrary ways. What would you suggest? The hook could be near the bottom instead, right before the definition of $s, with lots more parameters. By that point, some of the original parameters (like $text) have been overridden, but that could be changed.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com wrote:
Okay, that makes sense - there's probably no reason to allow more than one hook to override the output. The one downside of this approach that I can see is that functions that don't override the output might not get called at all, if they're later in the array than a function that does override it. I don't think that's a big deal, since I can't imagine some large amount of custom code called for every broken link, or two extensions "collaborating" on creating a new link. Unless anyone objects, I'll go with your suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false and alter some $ret variable to totally change things if they wanted, but exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified $text, for reference) could allow some extensions to coexist peacefully. If there's a conflict there's a conflict, of course, but we may as well try to avoid that a bit more.
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that list. Would the call to wfRunHooks() be placed before or after the line that sets the actual link ($s)? If it comes afterwards, passing in the variables of $style and $inside might not be necessary, since those are just helper variables that are used to create the link. Maybe it's also better to not have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values could be passed in without problems (and the new variables could similarly be ignored).
-Yaron
On Wed, Mar 5, 2008 at 12:43 PM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com wrote:
Okay, that makes sense - there's probably no reason to allow more than
one
hook to override the output. The one downside of this approach that I
can
see is that functions that don't override the output might not get
called at
all, if they're later in the array than a function that does override
it. I
don't think that's a big deal, since I can't imagine some large amount
of
custom code called for every broken link, or two extensions
"collaborating"
on creating a new link. Unless anyone objects, I'll go with your
suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false and alter some $ret variable to totally change things if they wanted, but exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified $text, for reference) could allow some extensions to coexist peacefully. If there's a conflict there's a conflict, of course, but we may as well try to avoid that a bit more.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Thanks Yaron for taking on the task of adding this hook :)
One use case could be for integration with something like Brion's TitleKey extension, so if the title already exists, but with a different capitalization, a known link obj could be generated rather than a broken link obj.
Say for example you're using TitleKey with an additional ArticleFromTitle hook to automatically resolve case differences (perhaps by 301 redirecting), and the article "WiMAX" already exists. When someone links to [[wimax]] - it would be nice to be able to show that as a known link obj.
-- Jim R. Wilson (jimbojw)
On Wed, Mar 5, 2008 at 11:59 AM, Yaron Koren yaron57@gmail.com wrote:
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that list. Would the call to wfRunHooks() be placed before or after the line that sets the actual link ($s)? If it comes afterwards, passing in the variables of $style and $inside might not be necessary, since those are just helper variables that are used to create the link. Maybe it's also better to not have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values could be passed in without problems (and the new variables could similarly be ignored).
-Yaron
On Wed, Mar 5, 2008 at 12:43 PM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com wrote:
Okay, that makes sense - there's probably no reason to allow more than
one
hook to override the output. The one downside of this approach that I
can
see is that functions that don't override the output might not get
called at
all, if they're later in the array than a function that does override
it. I
don't think that's a big deal, since I can't imagine some large amount
of
custom code called for every broken link, or two extensions
"collaborating"
on creating a new link. Unless anyone objects, I'll go with your
suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false and alter some $ret variable to totally change things if they wanted, but exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified $text, for reference) could allow some extensions to coexist peacefully. If there's a conflict there's a conflict, of course, but we may as well try to avoid that a bit more.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I would like to say thank you for adding going through this as well :)
regarding jimbojw's words:
I would agree it could be nice to have case-diff links be known link objects, however, though I can't think of an example atm, I'm sure there are articles with the same name but different cases - and then it becomes a problem of trying to understand which is the one the user meant. Places like search, where TitleKey makes a lot of sense, let the user make a conscious choice by showing him all of the options.
--Wiredtape
Jim Wilson wrote:
Thanks Yaron for taking on the task of adding this hook :)
One use case could be for integration with something like Brion's TitleKey extension, so if the title already exists, but with a different capitalization, a known link obj could be generated rather than a broken link obj.
Say for example you're using TitleKey with an additional ArticleFromTitle hook to automatically resolve case differences (perhaps by 301 redirecting), and the article "WiMAX" already exists. When someone links to [[wimax]] - it would be nice to be able to show that as a known link obj.
-- Jim R. Wilson (jimbojw)
On Wed, Mar 5, 2008 at 11:59 AM, Yaron Koren yaron57@gmail.com wrote:
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that list. Would the call to wfRunHooks() be placed before or after the line that sets the actual link ($s)? If it comes afterwards, passing in the variables of $style and $inside might not be necessary, since those are just helper variables that are used to create the link. Maybe it's also better to not have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values could be passed in without problems (and the new variables could similarly be ignored).
-Yaron
On Wed, Mar 5, 2008 at 12:43 PM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com wrote:
Okay, that makes sense - there's probably no reason to allow more than
one
hook to override the output. The one downside of this approach that I
can
see is that functions that don't override the output might not get
called at
all, if they're later in the array than a function that does override
it. I
don't think that's a big deal, since I can't imagine some large amount
of
custom code called for every broken link, or two extensions
"collaborating"
on creating a new link. Unless anyone objects, I'll go with your
suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false and alter some $ret variable to totally change things if they wanted, but exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified $text, for reference) could allow some extensions to coexist peacefully. If there's a conflict there's a conflict, of course, but we may as well try to avoid that a bit more.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Also, as a side note: Yaron, is there some forum or mailing list for Semantic Forms?
--Wiredtape
Ben wrote:
I would like to say thank you for adding going through this as well :)
regarding jimbojw's words:
I would agree it could be nice to have case-diff links be known link objects, however, though I can't think of an example atm, I'm sure there are articles with the same name but different cases - and then it becomes a problem of trying to understand which is the one the user meant. Places like search, where TitleKey makes a lot of sense, let the user make a conscious choice by showing him all of the options.
--Wiredtape
Jim Wilson wrote:
Thanks Yaron for taking on the task of adding this hook :)
One use case could be for integration with something like Brion's TitleKey extension, so if the title already exists, but with a different capitalization, a known link obj could be generated rather than a broken link obj.
Say for example you're using TitleKey with an additional ArticleFromTitle hook to automatically resolve case differences (perhaps by 301 redirecting), and the article "WiMAX" already exists. When someone links to [[wimax]] - it would be nice to be able to show that as a known link obj.
-- Jim R. Wilson (jimbojw)
On Wed, Mar 5, 2008 at 11:59 AM, Yaron Koren yaron57@gmail.com wrote:
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that list. Would the call to wfRunHooks() be placed before or after the line that sets the actual link ($s)? If it comes afterwards, passing in the variables of $style and $inside might not be necessary, since those are just helper variables that are used to create the link. Maybe it's also better to not have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values could be passed in without problems (and the new variables could similarly be ignored).
-Yaron
On Wed, Mar 5, 2008 at 12:43 PM, Simetrical Simetrical+wikilist@gmail.com wrote:
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com wrote:
Okay, that makes sense - there's probably no reason to allow more than
one
hook to override the output. The one downside of this approach that I
can
see is that functions that don't override the output might not get
called at
all, if they're later in the array than a function that does override
it. I
don't think that's a big deal, since I can't imagine some large amount
of
custom code called for every broken link, or two extensions
"collaborating"
on creating a new link. Unless anyone objects, I'll go with your
suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false and alter some $ret variable to totally change things if they wanted, but exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified $text, for reference) could allow some extensions to coexist peacefully. If there's a conflict there's a conflict, of course, but we may as well try to avoid that a bit more.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Ben: yeah, there's a Google Group for it, at http://groups.google.com/group/semantic-forms.
Thinking it over again, maybe that nine or ten arguments really is necessary after all. In the most extreme case, you could have one extension that turns different broken links different colors, another that redirects mis-capitalized links, and another that handles a custom value in the link's query string, and you would want them all to work together. Let me think about this, and I'll come back with another proposal.
-Yaron
On Wed, Mar 5, 2008 at 1:39 PM, Ben chuwiey@gmail.com wrote:
Also, as a side note: Yaron, is there some forum or mailing list for Semantic Forms?
--Wiredtape
Ben wrote:
I would like to say thank you for adding going through this as well :)
regarding jimbojw's words:
I would agree it could be nice to have case-diff links be known link objects, however, though I can't think of an example atm, I'm sure there are articles with the same name but different cases - and then it becomes a problem of trying to understand which is the one the user meant. Places like search, where TitleKey makes a lot of sense, let the user make a conscious choice by showing him all of the options.
--Wiredtape
Jim Wilson wrote:
Thanks Yaron for taking on the task of adding this hook :)
One use case could be for integration with something like Brion's TitleKey extension, so if the title already exists, but with a different capitalization, a known link obj could be generated rather than a broken link obj.
Say for example you're using TitleKey with an additional ArticleFromTitle hook to automatically resolve case differences (perhaps by 301 redirecting), and the article "WiMAX" already exists. When someone links to [[wimax]] - it would be nice to be able to show that as a known link obj.
-- Jim R. Wilson (jimbojw)
On Wed, Mar 5, 2008 at 11:59 AM, Yaron Koren yaron57@gmail.com wrote:
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that
list.
Would the call to wfRunHooks() be placed before or after the line
that sets
the actual link ($s)? If it comes afterwards, passing in the
variables of
$style and $inside might not be necessary, since those are just
helper
variables that are used to create the link. Maybe it's also better to
not
have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values
could be
passed in without problems (and the new variables could similarly be ignored).
-Yaron
On Wed, Mar 5, 2008 at 12:43 PM, Simetrical <
Simetrical+wikilist@gmail.com>
wrote:
On Wed, Mar 5, 2008 at 12:33 PM, Yaron Koren yaron57@gmail.com
wrote:
Okay, that makes sense - there's probably no reason to allow
more than
one
hook to override the output. The one downside of this approach
that I
can
see is that functions that don't override the output might not
get
called at
all, if they're later in the array than a function that does
override
it. I
don't think that's a big deal, since I can't imagine some large
amount
of
custom code called for every broken link, or two extensions
"collaborating"
on creating a new link. Unless anyone objects, I'll go with your
suggestion.
Putting it at the bottom with more parameters seems like a better idea, on second thought. I'd still permit hooks to return false
and
alter some $ret variable to totally change things if they wanted,
but
exposing the parameters $u, $style, $prefix, the modified $text, $inside, and $trail (as well as $nt, $query, and the unmodified
$text,
for reference) could allow some extensions to coexist peacefully.
If
there's a conflict there's a conflict, of course, but we may as
well
try to avoid that a bit more.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Wed, Mar 5, 2008 at 12:59 PM, Yaron Koren yaron57@gmail.com wrote:
Oh, wow - 10 parameters for the hook function? Somehow that seems like overkill, though I don't know what exactly could be removed from that list.
SkinTemplateTabAction has 9. So does ArticleSaveComplete. Those seem to be the biggest; up for record-breaking? :D
Would the call to wfRunHooks() be placed before or after the line that sets the actual link ($s)? If it comes afterwards, passing in the variables of $style and $inside might not be necessary, since those are just helper variables that are used to create the link.
Definitely before. Otherwise you'd have to parse $s if you wanted to tweak some single aspect of the link.
Maybe it's also better to not have makeBrokenLinkObj() override the values of $text and $trail, and instead use new variables for those, so that the original values could be passed in without problems (and the new variables could similarly be ignored).
Well, it's not really necessary. The original $trail is just "$trail$inside", I think. (Looking at the code, actually, it depends on $wgContLang->linkTrail(), but I think that's always *supposed* to be the case.) And $text is equal to the input $text unless there was no input, in which case it just has a default that needs to be specified in the code instead of on the call line. Passing &$this, $nt, $query, &$u, &$style, &$prefix, &$text, &$inside, &$trail, &$ret should be fine.
On Wed, Mar 5, 2008 at 1:13 PM, Jim Wilson wilson.jim.r@gmail.com wrote:
One use case could be for integration with something like Brion's TitleKey extension, so if the title already exists, but with a different capitalization, a known link obj could be generated rather than a broken link obj.
That would be much better done as a hook in makeLinkObj(). You're running extra queries for no good reason if you wait till it's already been decided broken, and you might break other things too. This hook should be for if the link is really broken, and you just want to handle broken links differently than linking to an edit page or something.
This is the biggest discussion I've ever seen about adding a single stupid hook. :)
Simectrical said:
That would be much better done as a hook in makeLinkObj(). You're running extra queries for no good reason if you wait till it's already been decided broken, and you might break other things too.
Yeah, i guess that makes sense - maybe we need two record breaking hooks? :)
-- Jim
On Thu, Mar 6, 2008 at 9:44 AM, Yaron Koren yaron57@gmail.com wrote:
This is the biggest discussion I've ever seen about adding a single stupid hook. :)
Well, maybe we can break two records at once. :) Okay, I guess I'll add the hook in later today, following your specifications.
-Yaron
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
The hook is now checked in to Linker.php, and I updated docs/hooks.txt as well.
-Yaron
On Thu, Mar 6, 2008 at 12:38 PM, Jim Wilson wilson.jim.r@gmail.com wrote:
Simectrical said:
That would be much better done as a hook in makeLinkObj(). You're running extra queries for no good reason if you wait till it's already been decided broken, and you might break other things too.
Yeah, i guess that makes sense - maybe we need two record breaking hooks? :)
-- Jim
On Thu, Mar 6, 2008 at 9:44 AM, Yaron Koren yaron57@gmail.com wrote:
This is the biggest discussion I've ever seen about adding a single stupid hook. :)
Well, maybe we can break two records at once. :) Okay, I guess I'll add
the
hook in later today, following your specifications.
-Yaron
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org