r65114 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r65113‎ | r65114 | r65115 >
Date:10:19, 16 April 2010
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Fix hooks.txt SpecialSearchNogomatch $title to &$title

* (bug 23206) Add Special::Search hook for detecting successful "Go"

Not implemented completely as per patch, $term removed, seems a bit useless/redundant (and doesn't exist for Nomatch)
Modified paths:
  • /trunk/phase3/CREDITS (modified) (history)
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/specials/SpecialSearch.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -1498,9 +1498,13 @@
14991499 $opts: FormOptions for this request
15001500 &$query_options: array of options for the database request
15011501
 1502+'SpecialSearchGomatch': called when user clicked the "Go" button and the target
 1503+exists
 1504+&$title: title object generated from the text entred by the user
 1505+
15021506 'SpecialSearchNogomatch': called when user clicked the "Go" button but the
15031507 target doesn't exist
1504 -$title: title object generated from the text entred by the user
 1508+&$title: title object generated from the text entred by the user
15051509
15061510 'SpecialSearchProfiles': allows modification of search profiles
15071511 &$profiles: profiles, which can be modified.
Index: trunk/phase3/CREDITS
@@ -75,6 +75,7 @@
7676 * Brianna Laugher
7777 * Carlin
7878 * Conrad Irwin
 79+* Dan Barrett
7980 * Dan Nessett
8081 * Daniel Arnold
8182 * Denny Vrandecic
Index: trunk/phase3/includes/specials/SpecialSearch.php
@@ -92,6 +92,7 @@
9393 # If there's an exact or very near match, jump right there.
9494 $t = SearchEngine::getNearMatch( $term );
9595 if( !is_null( $t ) ) {
 96+ wfRunHooks( 'SpecialSearchGomatch', array( &$t ) );
9697 $wgOut->redirect( $t->getFullURL() );
9798 return;
9899 }
@@ -881,7 +882,6 @@
882883 $bareterm = substr( $term, strpos( $term, ':' ) + 1 );
883884 }
884885
885 -
886886 $profiles = $this->getSearchProfiles();
887887
888888 // Outputs XML for Search Types
Index: trunk/phase3/RELEASE-NOTES
@@ -47,6 +47,7 @@
4848 escaping.
4949 * Special:Listfiles now supports a username parameter
5050 * Special:Random carries over query string parameters
 51+* (bug 23206) Add Special::Search hook for detecting successful "Go"
5152
5253 === Bug fixes in 1.17 ===
5354 * (bug 17560) Half-broken deletion moved image files to deletion archive

Follow-up revisions

RevisionCommit summaryAuthorDate
r78268Followup r65114 (per CR cabal), remove SpecialSearchGomatch, replace with Spe...reedy18:57, 12 December 2010

Comments

#Comment by Bryan (talk | contribs)   11:35, 16 April 2010

I think it would be better to move the hook a level up directly under the SearchEngine::getNearMatch and rename it SpecialSearchGo. That is more flexible.

#Comment by Werdna (talk | contribs)   01:22, 9 December 2010

Renaming hooks is very rarely a good idea.

#Comment by Bryan (talk | contribs)   07:47, 9 December 2010

Until a hook has appeared into a full release I see no problem with renaming.

#Comment by 😂 (talk | contribs)   15:47, 12 December 2010

If it hasn't been released yet, we should rename it and move it to a better spot.

Werdna's right: renaming it after release is a bad idea.

#Comment by Bryan (talk | contribs)   18:30, 12 December 2010

Well, 1.17 hasn't been released yet, so we still can change it.

#Comment by Reedy (talk | contribs)   18:16, 12 December 2010

Hmm, looking at SearchEngine::getNeraMatch, it has

wfRunHooks( 'SearchGetNearMatchComplete', array( $searchterm, &$title ) );

done after

$title = self::getNearMatchInternal( $searchterm );

Do we need to move the hook? (Or even keep this hook, delete it).. Or do we want to call it before getNearMatchInternal? Seems a bit redundant to me then...

#Comment by Bryan (talk | contribs)   18:29, 12 December 2010

The SearchGetNearMatchComplete is for backend operations, with the most likely use case overwriting the title object. The SpecialSearch* hooks though are for UI modification: adding text to the output, canceling the redirection, etc. They are not redundant in my opinion.

Status & tagging log