Problem/Motivation

search.module contains lots of procedural code that should be refactored.

Proposed resolution

Create a SearchText class / search.text service to use instead of the procedural code.

Remaining tasks

User interface changes

None

API changes

  • search_invoke_preprocess() to \Drupal\search\SearchText::invokePreprocess()
  • search_index_split() to \Drupal\search\SearchText::indexSplit()
  • search_expand_cjk() to \Drupal\search\SearchText::expandCjk()
  • search_simplify() to \Drupal\search\SearchText::simplify()

Data model changes

None

CommentFileSizeAuthor
#6 2719961-6.patch24.32 KBalexpott
#6 2-6-interdiff.txt3.86 KBalexpott
#2 2719961-2.patch24.32 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2719961-2.patch24.32 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +Needs subsystem maintainer review

I think we should also consider moving search_excerpt and _search_find_match_with_simplify() here too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: 2719961-2.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2-6-interdiff.txt3.86 KB
new 2719961-6.patch24.32 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/search/search.module
@@ -10,61 +10,37 @@
+ * @deprecated in Drupal 8.1.0 for removal before Drupal 9.0.0. Use
+ *   SEARCH_PREG_CLASS_NUMBERS instead.
...
+ * @deprecated in Drupal 8.1.0 for removal before Drupal 9.0.0. Use
+ *   SEARCH_PREG_CLASS_PUNCTUATION instead.

Suppose new constants should be FQN interface

Status: Needs review » Needs work

The last submitted patch, 6: 2719961-6.patch, failed testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented 
+++ b/core/modules/search/search.module
@@ -10,61 +10,37 @@
+define('PREG_CLASS_NUMBERS', SearchTextInterface::PREG_CLASS_NUMBERS);
...
+define('PREG_CLASS_PUNCTUATION', SearchTextInterface::PREG_CLASS_PUNCTUATION);
...
+define('PREG_CLASS_CJK', SearchTextInterface::PREG_CLASS_CJK);

These preg constants could be on Unicode instead, and then everyone could use them.

Not a blocker by any means, though.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I like the idea of this... The tests are not passing currently though. Not sure why, but the failures look to be real.

Some nitpicks/suggestions on the patch itself:

  1. I agree with #9 that the constants would be better on the Unicode class/interface. Search is already using some of those constants anyway.
  2. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+  /**
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
+
+  /**
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
+  /**
+   * @var \Drupal\Component\Transliteration\TransliterationInterface
+   */
+  protected $transliteration;
+
+  /**
+   * @var array
+   */
+  protected $truncated = [];
+
+  protected $lastIndexSplitText;
+  protected $lastIndexSplit;
+

    This whole section needs docs. 

  3. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+  public function __construct(ModuleHandlerInterface $module_handler, ConfigFactoryInterface $config_factory, TransliterationInterface $transliteration) {

    No docs here either 

  4. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+   * @inheritDoc

    Missing {} around @inheritdoc 

  5. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+   * @inheritDoc

    same here, missing {} 

  6. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+   * @inheritDoc

    here too 

  7. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+   * @inheritDoc

    and here. 

  8. +++ b/core/modules/search/src/SearchText.php
@@ -0,0 +1,172 @@
+   * Helper function for array_walk in search_index_split.

    This is not the correct documentation any more.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

#3075703: Move search text processing to a service is a duplicate. Not sure which to close.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think better to close this one as duplicate because #3075703: Move search text processing to a service has green patch just missing constants conversion

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Closed (duplicate)
Related issues: +#3075703: Move search text processing to a service

Closing as duplicate of #3075703: Move search text processing to a service