wagtail-nest / wagtail-whoosh

Search backend for Wagtail CMS using Whoosh engine.
24 stars 8 forks source link

Features/language support #11

Closed michael-yin closed 5 years ago

michael-yin commented 5 years ago

Hi, @tjwalch

Thanks for your PR

I think it is better to only let user to config language code just like Postgres search backend.

So I did some modification, you can review the code and let me know if you have any question.

Thx

tjwalch commented 5 years ago

There are many cases when the built in analyzers are not enough. E g they don't handle Swedish extra vowels (åäö) correctly. Therefor it is necessary to be able to configure a custom analyzer, or the package is useless. Also I know postgres backend uses SEARCH_CONFIG but it is utterly non-descriptive. Better to just call it LANGUAGE then. And it should default to LANGUAGE_CODE, as that IS the default case. To not honor that is to make the library more stupid than it is. The default analyzer w/o any language set is not suitable for web page content anyway.

michael-yin commented 5 years ago

@tjwalch

Thanks for your clarification.

I have added ANALYZER support and modified the SEARCH_CONFIG to LANGUAGE.

I hope LANGUAGE can let more people try this package in easy way and ANALYZER can help if people need more control.

Thx.

tjwalch commented 5 years ago

This looks good now. It is probably the least confusing to do like this and check that the language is actually supported, even if Whoosh just skips unsupported languages. Otherwise someone might think they get language support and wonder why it isn't working! There is just one place in README.md where you forgot to replace SEARCH_CONFIG with LANGUAGE.

michael-yin commented 5 years ago

@tjwalch

Thanks

I will release a new version in this week.