Last modified: 2014-02-12 23:38:26 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T48611, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 46611 - Change settings to not override those set in LocalSettings.php or other custom setting sources
Change settings to not override those set in LocalSettings.php or other custo...
Status: PATCH_TO_REVIEW
Product: MediaWiki extensions
Classification: Unclassified
SphinxSearch (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Svemir Brkic
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-27 18:26 UTC by Alexia E. Smith
Modified: 2014-02-12 23:38 UTC (History)
2 users (show)

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


Attachments
Modified files ready to go. (174.78 KB, application/x-gzip)
2013-03-27 18:26 UTC, Alexia E. Smith
Details

Description Alexia E. Smith 2013-03-27 18:26:26 UTC
Created attachment 11998 [details]
Modified files ready to go.

Various fixes to improve this extension.  It has not been updated since 2011-09-11 and is current incompatible with Mediawiki 1.19.x and 1.20.x.  It also improves the settings interface to allow it to be configured without modifying the extension.

diff --git a/SphinxMWSearch.php b/SphinxMWSearch.php
index edb08b8..e3e7547 100644
--- a/SphinxMWSearch.php
+++ b/SphinxMWSearch.php
@@ -179,14 +179,13 @@ class SphinxMWSearch extends SearchEngine {
        }
 
        /**
-    * Find snippet highlight settings for a given user
+  * Find snippet highlight settings for all users
         *
-    * @param $user User
         * @return Array contextlines, contextchars
         */
-   public static function userHighlightPrefs( &$user ) {
-           $contextlines = $user->getOption( 'contextlines', 2 );
-           $contextchars = $user->getOption( 'contextchars', 75 );
+ public static function userHighlightPrefs() {
+         $contextlines = 2; // Hardcode this. Old defaults sucked. :)
+         $contextchars = 75; // same as above.... :P
                return array( $contextlines, $contextchars );
        }
 
diff --git a/SphinxSearch.i18n.php b/SphinxSearch.i18n.php
index fce1668..45ebb27 100644
--- a/SphinxSearch.i18n.php
+++ b/SphinxSearch.i18n.php
@@ -21,8 +21,6 @@ $messages['en'] = array(
 $messages['qqq'] = array(
        'sphinxsearch-desc' => '{{desc|name=Sphinx Search|url=http://www.mediawiki.org/wiki/Extension:SphinxSearch}}',
        'sphinxPowered' => '$1 is replaced with a text "Sphinx" inside a link.',
-   'sphinxSearchFailed' => 'Unused at this time. Parameters:
-* $1 - ...',
 );
 
 /** Afrikaans (Afrikaans)
diff --git a/SphinxSearch.php b/SphinxSearch.php
index 2632250..d746461 100644
--- a/SphinxSearch.php
+++ b/SphinxSearch.php
@@ -46,16 +46,24 @@ if ( !class_exists( 'SphinxClient' ) ) {
 }
 
 # Host and port on which searchd deamon is running
-$wgSphinxSearch_host = '127.0.0.1';
-$wgSphinxSearch_port = 9312;
+if (!isset($wgSphinxSearch_host)) {
+ $wgSphinxSearch_host = '127.0.0.1';
+}
+if (!isset($wgSphinxSearch_port) and !is_numeric($wgSphinxSearch_port)) {
+ $wgSphinxSearch_port = 9312;
+}
 
 # Main sphinx.conf index to search
-$wgSphinxSearch_index = "wiki_main";
+if (!$wgSphinxSearch_index) {
+ $wgSphinxSearch_index = "wiki_main";
+}
 
 # By default, we search all available indexes
 # You can also specify them explicitly, e.g
 # $wgSphinxSearch_index_list = "wiki_main,wiki_incremental";
-$wgSphinxSearch_index_list = "*";
+if (!$wgSphinxSearch_index_list) {
+ $wgSphinxSearch_index_list = "*";
+}
 
 # If you have multiple index files, you can specify their weights like this
 # See http://www.sphinxsearch.com/docs/current.html#api-func-setindexweights
@@ -101,7 +109,7 @@ $wgSphinxSearchPersonalDictionary = '';
 
 # If true, use SphinxMWSearch for prefix search instead of the core default.
 # This influences results from ApiOpenSearch.
-$wgEnableSphinxPrefixSearch = false;
+$wgEnableSphinxPrefixSearch = true;
 
 function efSphinxSearchPrefixSetup() {
        global $wgHooks, $wgEnableSphinxPrefixSearch;
Comment 1 Alex Monk 2013-03-27 18:28:14 UTC
Please put this in Gerrit.
Comment 2 Svemir Brkic 2013-03-27 19:36:21 UTC
This line does not seem right:

+if (!isset($wgSphinxSearch_port) and !is_numeric($wgSphinxSearch_port)) {

Perhaps it should be:

+if (!isset($wgSphinxSearch_port) || !is_numeric($wgSphinxSearch_port)) {

But in general I would rather let the local administrator make sure if they are setting the port at all to make it correct. Quietly ignoring a bad setting is usually not a good idea.

These may cause notices on stricter configurations:

+if (!$wgSphinxSearch_index) {
+if (!$wgSphinxSearch_index_list) {

Should probably use isset like other suggested changes.

Finally, not sure why we would change default for $wgEnableSphinxPrefixSearch. Does that help with compatibility with 1.19 or 1.20 somehow?
Comment 3 Alexia E. Smith 2013-03-27 20:00:52 UTC
(In reply to comment #2)
> This line does not seem right:
> 
> +if (!isset($wgSphinxSearch_port) and !is_numeric($wgSphinxSearch_port)) {
> 
> Perhaps it should be:
> 
> +if (!isset($wgSphinxSearch_port) || !is_numeric($wgSphinxSearch_port)) {
> 
> But in general I would rather let the local administrator make sure if they
> are
> setting the port at all to make it correct. Quietly ignoring a bad setting is
> usually not a good idea.
> 
> These may cause notices on stricter configurations:
> 
> +if (!$wgSphinxSearch_index) {
> +if (!$wgSphinxSearch_index_list) {
> 
> Should probably use isset like other suggested changes.
> 
> Finally, not sure why we would change default for
> $wgEnableSphinxPrefixSearch.
> Does that help with compatibility with 1.19 or 1.20 somehow?

Sorry about that, I forgot to sanitize the code for Mediawiki's syntax rules.  It should be the && operator.  With || it could allow a setting slip through for the port that is not a numeric number thus passing bad information to the sphinxapi.php library.  Granted, you can pass garbage for the port to sphinxapi.php and it will automatically default to 9312 for the port.  Having the check at the extension level is so that the extension is acting on proper data.

I pushed out a new change that changes the default back for $wgEnableSphinxPrefixSearch and changes it to a isset() setting check.  We found that having this enabled to true improves search performance overall, but it makes more sense to keep it the old default with a setting check.
Comment 4 Alexia E. Smith 2013-03-27 20:01:29 UTC
(In reply to comment #1)
> Please put this in Gerrit.

Still learning some things, sorry!  Just figured out what I did wrong and why I could get it to push earlier.
https://gerrit.wikimedia.org/r/#/c/56202/
Comment 5 Svemir Brkic 2013-07-25 10:32:32 UTC
Just to add a note to thank Alexia for all the time and effort invested so far.

I was not dealing with the extension in a while, and MW processes changed a in the meantime.

I will try to figure out how the processes work now and will install everything locally again so that I can contribute to the discussion on gerrit.

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


Navigation
Links