-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optionally add trimmer to search pipeline #154
base: master
Are you sure you want to change the base?
Conversation
Note that adding the stopword filter to search isn't really necessary since those terms just won't be in the index. The trimmer on the other hand is really useful for the reason mentione above. But again ... this breaks compatibility with lunr.js so you probably shouldn't merge it! |
Updated this because the stopword filter actually isn't useful in the search pipeline. But the trimmer is! |
Updated again - the behaviour is disabled by default, but can be enabled with the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #154 +/- ##
==========================================
+ Coverage 96.02% 96.10% +0.07%
==========================================
Files 48 48
Lines 3171 3206 +35
==========================================
+ Hits 3045 3081 +36
+ Misses 126 125 -1 ☔ View full report in Codecov by Sentry. |
The question then is whether you want to have the option also add the stopword filter in earch - as mentioned it doesn't actually do anything, because those terms just won't be matched. Also I'm not sure what happens with multi-language models in that case. |
Fixes #151
but breaks bug-compatibility with lunr.jswhen an option is enabled (which also works, most of the time, in lunr.js with serialized models from lunr.py). The Javascript-side workaround is noted in olivernn/lunr.js#532 ... will lunr.js get updated? Magic 8-ball says "UNLIKELY"