From c6b4bdf0ab49388752a03ba4e65700eb0277143e Mon Sep 17 00:00:00 2001
From: David Causse <dcausse@wikimedia.org>
Date: Thu, 1 Aug 2019 11:46:24 +0200
Subject: [PATCH] Deprecate SearchEngine::userHighlightPrefs()

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                    |  4 ++++
 includes/search/SearchEngine.php      |  7 +++++--
 includes/search/SearchHighlighter.php | 23 ++++++++++++++++++++---
 includes/search/SqlSearchResult.php   |  9 +++------
 4 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 7cc0434032..744dffac6f 100644
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -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 ===
 * …
diff --git a/includes/search/SearchEngine.php b/includes/search/SearchEngine.php
index 31af13d5b0..151c0c60da 100644
--- a/includes/search/SearchEngine.php
+++ b/includes/search/SearchEngine.php
@@ -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 ];
 	}
 
diff --git a/includes/search/SearchHighlighter.php b/includes/search/SearchHighlighter.php
index 6c01f799d5..25799420e8 100644
--- a/includes/search/SearchHighlighter.php
+++ b/includes/search/SearchHighlighter.php
@@ -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
diff --git a/includes/search/SqlSearchResult.php b/includes/search/SqlSearchResult.php
index 25e87e70ff..9804e44d7d 100644
--- a/includes/search/SqlSearchResult.php
+++ b/includes/search/SqlSearchResult.php
@@ -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 );
 		}
 	}
 
-- 
2.20.1