Deprecate SearchEngine::userHighlightPrefs()
authorDavid Causse <dcausse@wikimedia.org>
Thu, 1 Aug 2019 09:46:24 +0000 (11:46 +0200)
committerDavid Causse <dcausse@wikimedia.org>
Thu, 1 Aug 2019 15:16:41 +0000 (17:16 +0200)
was only returning two constants and forced a cyclic dep bewteen
SearchEngine and its SearchResult instance.
Simple make these values a constant in SearchHighlighter and use
them as default values for the highlight methods.

Change-Id: Ia78408d5266d0a305006027fe6265a2a1d68b0b9

RELEASE-NOTES-1.34
includes/search/SearchEngine.php
includes/search/SearchHighlighter.php
includes/search/SqlSearchResult.php

index 7cc0434..744dffa 100644 (file)
@@ -424,6 +424,10 @@ because of Phabricator reports.
 * The following public properties on DatabaseBlock are deprecated: $mAuto,
   $mParentBlockId. To check for an autoblock use DatabaseBlock::getType; to
   check for the parent ID, use DatabaseBlock::getParentBlockId.
+* SearchEngine::userHighlightPrefs() is deprecated, simply stop passing
+  $contextlines and $contextchars to the SearchHighlighter methods, they will
+  use proper defaults defined in SearchHighlighter::DEFAULT_CONTEXT_LINES and
+  DEFAULT_CONTEXT_CHARS.
 
 === Other changes in 1.34 ===
 * …
index 31af13d..151c0c6 100644 (file)
@@ -433,10 +433,13 @@ abstract class SearchEngine {
        /**
         * Find snippet highlight settings for all users
         * @return array Contextlines, contextchars
+        * @deprecated in 1.34 use the SearchHighlighter constants directly
+        * @see SearchHighlighter::DEFAULT_CONTEXT_CHARS
+        * @see SearchHighlighter::DEFAULT_CONTEXT_LINES
         */
        public static function userHighlightPrefs() {
-               $contextlines = 2; // Hardcode this. Old defaults sucked. :)
-               $contextchars = 75; // same as above.... :P
+               $contextlines = SearchHighlighter::DEFAULT_CONTEXT_LINES;
+               $contextchars = SearchHighlighter::DEFAULT_CONTEXT_CHARS;
                return [ $contextlines, $contextchars ];
        }
 
index 6c01f79..2579942 100644 (file)
@@ -29,6 +29,9 @@ use MediaWiki\MediaWikiServices;
  * @ingroup Search
  */
 class SearchHighlighter {
+       const DEFAULT_CONTEXT_LINES = 2;
+       const DEFAULT_CONTEXT_CHARS = 75;
+
        protected $mCleanWikitext = true;
 
        /**
@@ -50,7 +53,12 @@ class SearchHighlighter {
         * @param int $contextchars
         * @return string
         */
-       public function highlightText( $text, $terms, $contextlines, $contextchars ) {
+       public function highlightText(
+               $text,
+               $terms,
+               $contextlines = self::DEFAULT_CONTEXT_LINES,
+               $contextchars = self::DEFAULT_CONTEXT_CHARS
+       ) {
                global $wgSearchHighlightBoundaries;
 
                if ( $text == '' ) {
@@ -507,7 +515,12 @@ class SearchHighlighter {
         * @param int $contextchars
         * @return string
         */
-       public function highlightSimple( $text, $terms, $contextlines, $contextchars ) {
+       public function highlightSimple(
+               $text,
+               $terms,
+               $contextlines = self::DEFAULT_CONTEXT_LINES,
+               $contextchars = self::DEFAULT_CONTEXT_CHARS
+       ) {
                $lines = explode( "\n", $text );
 
                $terms = implode( '|', $terms );
@@ -557,7 +570,11 @@ class SearchHighlighter {
         * @param int $contextchars Average number of characters per line
         * @return string
         */
-       public function highlightNone( $text, $contextlines, $contextchars ) {
+       public function highlightNone(
+               $text,
+               $contextlines = self::DEFAULT_CONTEXT_LINES,
+               $contextchars = self::DEFAULT_CONTEXT_CHARS
+       ) {
                $match = [];
                $text = ltrim( $text ) . "\n"; // make sure the preg_match may find the last line
                $text = str_replace( "\n\n", "\n", $text ); // remove empty lines
index 25e87e7..9804e44 100644 (file)
@@ -51,18 +51,15 @@ class SqlSearchResult extends SearchResult {
                global $wgAdvancedSearchHighlighting;
                $this->initText();
 
-               // TODO: make highliter take a content object. Make ContentHandler a factory for SearchHighliter.
-               list( $contextlines, $contextchars ) = $this->searchEngine->userHighlightPrefs();
-
                $h = new SearchHighlighter();
                if ( count( $this->terms ) > 0 ) {
                        if ( $wgAdvancedSearchHighlighting ) {
-                               return $h->highlightText( $this->mText, $this->terms, $contextlines, $contextchars );
+                               return $h->highlightText( $this->mText, $this->terms );
                        } else {
-                               return $h->highlightSimple( $this->mText, $this->terms, $contextlines, $contextchars );
+                               return $h->highlightSimple( $this->mText, $this->terms );
                        }
                } else {
-                       return $h->highlightNone( $this->mText, $contextlines, $contextchars );
+                       return $h->highlightNone( $this->mText );
                }
        }