Skip to content

Commit

Permalink
fix: handle phrases with apostrophes (#116)
Browse files Browse the repository at this point in the history
We're using shlex to split phrases. It uses the POSIX mode by default, which is
throwing an exception when there are unclosed quotes in phrases. This disables
the POSIX mode after an error, so the phrases can still be split correctly.
  • Loading branch information
Agrendalath authored May 4, 2022
1 parent 3cef5ef commit ad9cee4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
2 changes: 1 addition & 1 deletion edxsearch/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" Container module for testing / demoing search """

__version__ = '3.3.0'
__version__ = '3.4.0'
7 changes: 6 additions & 1 deletion search/result_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,12 @@ def excerpt(self):
return None

match_phrases = [self._match_phrase]
separate_phrases = list(shlex.split(self._match_phrase))
try:
separate_phrases = list(shlex.split(self._match_phrase))
# This can happen when the phrase contains an apostrophe - the POSIX mode does not allow unclosed quotations.
except ValueError:
separate_phrases = list(shlex.split(self._match_phrase, posix=False))

if len(separate_phrases) > 1:
match_phrases.extend(separate_phrases)
else:
Expand Down
24 changes: 24 additions & 0 deletions search/tests/test_search_result_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,30 @@ def test_excerpt_ellipsis_undecorated(self):
srp = SearchResultProcessor(test_result, 'Just a line')
self.assertIn(ELLIPSIS, srp.excerpt)

@ddt.data(
("shouldn't", "This <b>shouldn't</b> have failed."),
("shouldn\'t", " This <b>shouldn't</b> have failed."),
('"shouldn\'t have"', "This <b>shouldn't have</b> failed."),
("shouldn\\'t\\'ve", "<b>shouldn't've</b> failed."), # An even number of apostrophes needs to be escaped.
('"shouldn\'t\'ve failed"', "<b>shouldn't've failed</b>."),
("shou\'dn\'t\'ve", "This <b>shou'dn't've</b> failed."),
('"shou\'dn\'t\'ve failed"', "This <b>shou'dn't've failed</b>."),
)
@ddt.unpack
def test_excerpt_apostrophe(self, search_phrase, expected_excerpt):
test_result = {
"content": {
"notes": (
"This should not have failed. "
"This shouldn't have failed. "
"This shouldn't've failed. "
"This shou'dn't've failed."
)
}
}
srp = SearchResultProcessor(test_result, search_phrase)
self.assertIn(expected_excerpt, srp.excerpt)


class TestSearchResultProcessor(SearchResultProcessor):
"""
Expand Down

0 comments on commit ad9cee4

Please sign in to comment.