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

Handle case where data to be highlighted is None, and minor refactoring #156

Merged
merged 2 commits into from
Sep 6, 2020

Conversation

ajiteshr7
Copy link
Contributor

@ajiteshr7 ajiteshr7 commented Sep 6, 2020

  1. Handle case where data of the field to be highlighted is None,
    causing highlight method to fail since it expects data to be of string type. Fixes Highlighting fails with TypeError when highlighter_field value is None (while using HighlighterMixin) #155

  2. Taking out the extraction of terms from the request into a method of its own,
    namely get_terms so that its easier to override it with a custom implementation,
    adds to separation of concerns.

 Handle case where data of the field to be highlighted is None,
 causing highlight method to fail since it expects data to of string type.
…e method

Taking out the extraction of terms from request into a method of its own,
namely get_terms so that its easier to override it with a custom implementation,
adds to separation of concerns.
@ajiteshr7
Copy link
Contributor Author

ajiteshr7 commented Sep 6, 2020

There's one more issue with this highlighter, since it get the terms to highlight from the request data.
It actually also highlights page number present in the text, since when ?page=<page_number> is present in the query.
@rhblind

@coveralls
Copy link

coveralls commented Sep 6, 2020

Coverage Status

Coverage increased (+0.07%) to 88.606% when pulling ba350e0 on ajiteshr7:master into bb5bdb5 on rhblind:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 88.006% when pulling ba350e0 on ajiteshr7:master into bb5bdb5 on rhblind:master.

@rhblind
Copy link
Owner

rhblind commented Sep 6, 2020

Beautiful, thank you very much!

@rhblind rhblind merged commit 162e360 into rhblind:master Sep 6, 2020
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.

Highlighting fails with TypeError when highlighter_field value is None (while using HighlighterMixin)
3 participants