Skip to content
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

Fix default stemmer (again) #116

Merged
merged 2 commits into from Jul 19, 2021

Conversation

ViliusS
Copy link
Contributor

@ViliusS ViliusS commented Mar 28, 2021

TNTSearch changed default stemmer again teamtnt/tntsearch@b675c62.

To make our plugin behaviour consistent I would like to propose to remove 'default' value, use 'NoStemmer' as default and always set stemmer in the code.

Not sure if there is a better way to not break compatibility with older configs, it could produce errors, but everyone who have used 'default' previously should be aware about the change upstream anyway.

TNTSearch changed default stemmer again teamtnt/tntsearch@b675c62.

To make our plugin behaviour consistent remove 'default' value, use 'NoStemmer' as default and always set stemmer in the code.
@rhukster
Copy link
Member

I think that we still need to handle the case where stemmer is set to default as this was the default before and people could well have saved this value.. So if default use noStemmer ?

@ViliusS
Copy link
Contributor Author

ViliusS commented Mar 29, 2021

I can re-add

      if ($this->options['stemmer'] == 'default') {
           $indexer->setLanguage('no');
       } else {
           $indexer->setLanguage($this->option['stemmer']);
       }

to setIndex(). Is this enough or do I need to change it in other places?

@rhukster
Copy link
Member

I think that would be fine.

@ViliusS
Copy link
Contributor Author

ViliusS commented Mar 29, 2021

Done.

@ViliusS
Copy link
Contributor Author

ViliusS commented May 5, 2021

Just a reminder to merge :)

@w00fz w00fz requested a review from rhukster May 6, 2021 05:21
@ViliusS ViliusS mentioned this pull request Jul 19, 2021
@ViliusS
Copy link
Contributor Author

ViliusS commented Jul 19, 2021

@rhukster could you merge this and check if tntsearch library is up-to-date with upstream changes in https://github.com/teamtnt/tntsearch/pull/243/files ? Broken stemmers give people issues.

@rhukster rhukster merged commit 9620db3 into trilbymedia:develop Jul 19, 2021
@rhukster
Copy link
Member

done! sorry slipped through the cracks.

@ViliusS
Copy link
Contributor Author

ViliusS commented Jul 20, 2021

Thank you! Could you also update tntsearch library to the latest version?

@ViliusS ViliusS deleted the fix/change-default-stemmer branch July 20, 2021 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants