Support word stems (Porter Stemming)

Thanks a lot for proposing this addition!
You are right, having (English) stemming available for users of the database backend (or other "primitive" backends) would be great!
However, the extra step of downloading the external "library" is a bit of a drag. The algorithm is available publicly and easy enough to implement, so why not just add your own implementation in a processor? (Or ask Richard Heyes whether he wants to contribute his code to this project – might be even quicker.) Would be far better UX, and not too much additional effort.
Furthermore, the current patch introduces a dependency on the Libraries module, which I'd like to avoid. It also adds an undocumented new key for processors (but that wouldn't be as much of a problem – but also can be easily implemented differently).

So, would it be possible for you to just implement the algorithm directly in your processor (or find a GPL-compatible version which we can use)?

Awesome, thanks a lot!

Great job, thanks a lot!
However, while the implementation looks great, the code doesn't really follow Drupal's coding standards. I fixed that and also some code that looked more like a direct C port than actual PHP code.
I also attach a patch for your Github repository. I ran the tests there, and one of my changes seems to have fixed one of the three failures the test script reported for me. (Does it report any for you?) So even if you don't like the code style changes, maybe you can use some of the other changes.

Anyways, please take a look at my changes and tell me if you're fine with them. Then I guess I can commit this, and we'll finally have a stemmer processor right in the module. (Should then port ASAP to D8, too.)

In any case, thanks a lot again!

No, I didn't know that, must have forgot about writing those changes back to the module, too. Thanks a lot for catching that!
However, why did you also revert all my changes to the processor class?
Also, please create patches relative to the module directory, not to the Drupal installation – otherwise the test bot can't apply them.

Anyways, this should now be the final patch (interdiff compared to #9). Would you say this is RTBC now?

Getting your help for porting this to Drupal 8 would of course be awesome, thanks a lot!
For D8, though, we should definitely also add tests, we have some for all processors there. (But you can leave that part for someone else, too, no worries.)

Hey all,

Just applied this patch to my Search API config and it seems to be working as advertised. I tested this locally with the following:

Mac OSX 10.10.5 (Yosemite)
MAMP 4.0.5
PHP 5.6.25
MySQL 5.6.28

After configuring Search API to use the "Stem Words" processor, and re-indexing all my nodes, I ran a search for "teach" and several results with teach, teacher, teaching were at the top. I would suggest, as a feature request or enhancement, that Search API could highlight the word AND the stemmed results. That would make it more apparent to the user why certain results are appearing.

I'll leave it up to @drunken-monkey to determine if this issue should be considered RTBC.

Yes, looks good. Thanks for testing!
Regarding highlighting: Unfortunately, that's not really possible to do without a completely different framework (like Apache Solr has).

Anyways: committed.
Again, thank you very much for contributing this! Great job!

Great job, thanks a lot!
Didn't test it, but from looking at the code, I have a few smaller notes:

• In Drupal 8, every class has to go into its own file. Also, we use namespaces there for avoiding conflicts, not class name prefixes.
• I prefer porter_stemmer for the plugin ID, to reflect the class name. (Or change both to just "stemmer", since that's also the label? Or change the label?)
• The weights for the preprocessing stages were too low; this should definitely run after the tokenizer.
• The processor also needs a config schema.
• Why go to the trouble of splitting the "Exceptions" string and calling parse_ini_string() for each line individually? Why not just call it on the whole?
• More importantly (don't know how I missed that for D7): Why not parse this once, when submitting the config form, instead of every time the processor is called? The way it currently is, you can't even report a syntax error to the user and end up with invalid config, that leads to warnings on every search or indexing operation. (Didn't implement this change in my patch yet.)
• We should also clearly note in the description (or maybe even in the label?) that this stemmer only works for English.
• Relatedly, we might want to ignore any items that have a different language set. (Also not implemented.)

Except for the two noted exceptions, all of these are fixed in the attached patch.
Also, the tests are still missing. (But pretty easy to add: see, for instance, \Drupal\Tests\search_api\Unit\Plugin\Processor\IgnoreCaseTest. You just need a sufficiently long list of correct stems – but you already have those in your test on Github, it seems. I guess you can just copy the full.ini from there to our module. Or, maybe just a subset, since that file is pretty large.)

Thanks @mark_fullmer for that test, it's great! I now have a better understanding of how a stemmer is supposed to work, great work!

I did find some nitpicks, some docs issues, a question and a request for additional test.

1. +++ b/src/Plugin/search_api/processor/Resources/Porter2.php
   @@ -0,0 +1,652 @@
   +/**
   + * Implements the Porter2 stemming algorithm.
   + *
   + * @see https://github.com/markfullmer/porter2
   + */
   +class Porter2 {
   

   I wanted to ask to actually use that library as a dependency, but that doesn't seem like a good idea, we wouldn't get any of their tests (as there's only one) and we'd need to contribute at least a composer.json and namespacing. So let's not.

   We should probably do a // @codingStandardsIgnoreFile line at the top of this file so phpcs doesn't complain about it though.

2. +++ b/src/Plugin/search_api/processor/Stemmer.php
   @@ -0,0 +1,147 @@
   + *   description = @Translation("Stems search terms (e.g., <em>talking</em> to <em>talk</em>). Currently, this only acts on English language content. It uses the Porter 2 stemmer algorithm (<a href=""https://wikipedia.org/wiki/Stemming"">More information</a>). For best results, use after tokenizing."),
   

   The "" here doesn't make much sense, let's use proper escaping \" or swap the quotes for the entire @Translation to use '

3. +++ b/src/Plugin/search_api/processor/Stemmer.php
   @@ -0,0 +1,147 @@
   +   * @var array
   +   */
   

   @var string[]
     An array of stemmed words, keyed by original word.

4. +++ b/src/Plugin/search_api/processor/Stemmer.php
   @@ -0,0 +1,147 @@
   +   * @var string[]
   

   Same comment here?

5. +++ b/src/Plugin/search_api/processor/Stemmer.php
   @@ -0,0 +1,147 @@
   +      // Limit this processor to English language data.
   +      if ($item->getLanguage() == 'en') {
   +        foreach ($item->getFields() as $name => $field) {
   +          if ($this->testField($name, $field)) {
   +            $this->processField($field);
   

   This is simple code, but nested a bit too deep for my taste. Can we do something like this to reduce nesting?

   // Skip items that are not in English.
   if ($item->getLanguage() !== 'en') {
   continue;
   }
   
   foreach (...
   

   This is just cosmetics, so feel free to disagree.

6. +++ b/tests/src/Unit/Plugin/Processor/StemmerTest.php
   @@ -0,0 +1,137 @@
   +  public function testProcess($passed_value, $expected_value) {
   

   This test is great! It'd be lovely to also test the preprocessIndexItems method with at least two items (english / not english) to see that that also works.

Wow, great job, Mark, thanks a lot!

We should probably do a // @codingStandardsIgnoreFile line at the top of this file so phpcs doesn't complain about it though.

I disagree. If we include the code in the project, we should also format it according to the Drupal coding standards.
Also, do you see any violations? I went through the file and fixed all I could find.

The "" here doesn't make much sense, let's use proper escaping \" or swap the quotes for the entire @Translation to use '

"" is the proper escaping for annotations. As far as I can see, there is no other way. (Otherwise, I'd prefer either of your mentioned alternatives, too.)

@var string[]
  An array of stemmed words, keyed by original word.

That's the wrong format for properties.

I totally agree with your items 5 and 6, though. Should be fixed in the attached patch.

Thanks for correcting me on those points, changes look good!

Great to hear, thanks again for reviewing.
And of course thanks a lot again to Mark for all his work here!
Committed.