Problem/Motivation

The interface Drupal\Core\Plugin\PluginFormInterface was added to core since the search plugin patch was first devised. Because that wasn't available during the initial plugin conversion, one-off form methods were added to SearchInterface instead.

Proposed resolution

We should remove the one-off form methods and use this now-standard interface for greater consistency.

Remaining tasks

N/A

User interface changes

N/A

API changes

  • Two methods are removed from Drupal\search\Plugin\SearchInterface
  • NodeSearch and SearchExtraTypeSearch methods are changed to conform to PluginFormInterface
  • SearchSettingsForm only invokes form methods on plugins that implement PluginFormInterface

#2003482: Convert hook_search_info to plugin system

CommentFileSizeAuthor
#10 search-2086201-10.patch8.33 KBpwolanin
#5 search-2086201-5.patch8.33 KBtim.plunkett
#1 search-2086201-1.patch8.32 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
Issue tags: +Plugin system, +Plugin-conversion
StatusFileSize
new search-2086201-1.patch8.32 KB

This is what I had in mind.

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community

The relevant search module tests pass for me locally, and I think this is a good cleanup.

pwolanin’s picture

pwolanin commented

#1: search-2086201-1.patch queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

Sorry, no longer applies. Seems like a good change though.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new search-2086201-5.patch8.33 KB

Trivial reroll:

+++ b/core/modules/search/lib/Drupal/search/Form/SearchSettingsForm.php
@@ -11,6 +11,7 @@
 use Drupal\system\SystemConfigFormBase;

This was moved.

Status: Reviewed & tested by the community » Needs work
Issue tags: -Plugin system, -Plugin-conversion

The last submitted patch, search-2086201-5.patch, failed testing.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
Issue tags: +Plugin system, +Plugin-conversion

#5: search-2086201-5.patch queued for re-testing.

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

pwolanin’s picture

pwolanin commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
StatusFileSize
new search-2086201-10.patch8.33 KB

patch was able to apply it with fuzz, so just a simple re-roll.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 34f0d6d and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.