I just implemented (and committed) #2912667: Adopt to proposed changes in Search API to improve highlighting.
But I think the implementation that was introduced by #2278433: Add option to Views field handlers to use the "highlighted_fields" extra data is erroneous because the Highlight processor only applies the backend values if he finds matches himself. And if that's the case, the backend values are just appended.
I suggest to only run the Highlight processors own matching, if the backend doesn't deliver anything.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 2945560-10--highlight_keep_existing_values.patch | 5.03 KB | drunken monkey |
| #10 | 2945560-10--highlight_keep_existing_values--tests-only.patch | 2.71 KB | drunken monkey |
Comments
Comment #2
mkalkbrennerComment #3
mkalkbrennerUsing this patch I see highlighted fields in views that are highlighted based on multilingual stemming :-)
Comment #4
mkalkbrennerComment #5
drunken monkeyWhy do you even need the Highlight processor if the backend already returns highlighting? In such a case, I'd just suggest disabling the processor.
Comment #6
borisson_@mkalkbrenner, do you think we can test this without having multilingual stemming in the test?
At least, we can mimick this in the
\Drupal\Tests\search_api\Unit\Processor\HighlightTest.Comment #7
borisson_Same as #2945588: Processor plugins should expose their configs, I had this open for a while and didn't see the latest comment by @drunken monkey.
I agree with #5!
Comment #8
mkalkbrennerWith the patch from #2 the Highlighter processor works well. And you get the excerpt from it!
Having an excerpt isn't a native Solr feature. Using the Highlight processor is good solution and offers a unique interface for the user.
The same is true for the highlighting prefix and suffix. And for the configuration of which fields to highlight.
Why adding a dedicated config for Solr as you proposed in #2945588: Processor plugins should expose their configs that just duplicates everything?
BTW with that patch we're working in the direction of https://www.drupal.org/project/search_api_solr/issues/2718571#comment-11...
Comment #9
mkalkbrennerThe current issues with Solr 7.2 force me to release a third alpha right now before the first beta of Search API Solr Search 8.x-2.0 will be released.
Highlighting will only work if the Highlight Processor is enabled. But without the patch in #2 Highlighting will not work as expected in Views.
If you can't agree on my arguments in #8, I think I'll go ahead provide a dedicated processor within search_api_solr with beta1. But that would be somehow disappointing because that would be a step in a direction that complicates switching between the backends.
Anyway, I'm interested in your feedback on #8.
Comment #10
drunken monkeyArchitecturally, one plugin using another's configuration (and even across modules) isn't a very sound pattern. However, I guess it does make sense from a user's perspective, so if you want to go with that, sure, why not. It's just not a pattern we've used in the past, and I still don't think it's one we should encourage.
But, as said, it's your decision for the Solr backend, and the changes made here are definitely benign in any case. I just got rid of a bit overcomplicated code and even added test so Joris is happy – please test/review!
Comment #12
mkalkbrennerThank you Thomas!
The patch looks good and I already tested in a production environment, too.
Comment #13
drunken monkeyGood to hear, thanks for your feedback!
Committed.
Thanks again, everyone!