Remove "<" and ">" as they cannot be escaped #1415
Conversation
|
Thanks for the PR. I wonder if it had any side effects that we tried to escape it? All tests are still green ... Could you update the changelog? |
|
Changelog updated. As |
| // @see https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#_reserved_characters | ||
| $nonEscapableChars = ['<', '>']; | ||
|
|
||
| foreach ($nonEscapableChars as $char) { |
There was a problem hiding this comment.
I wonder if we should update line
Elastica/lib/Elastica/Util.php
Line 77 in e47dd0e
There was a problem hiding this comment.
I would drop the list of characters in the description of replaceBooleanWordsAndEscapeTerm. My reasoning is that it exposes the internal behaviour of the used submethods. The character list creates maintenance backlog as it is not something that the wrapper method replaceBooleanWordsAndEscapeTerm has control over at present.
Yet if you believe it adds value then it needs updating too. Are you looking at any particular wording?
There was a problem hiding this comment.
+1 on removing it. I'm good with most wordings :-)
There was a problem hiding this comment.
The method description is updated. I believe we can move this PR forward now.
|
@martin-georgiev Thanks a lot for the contribution. |
|
@ruflin It was my pleasure. |
<and>cannot be escaped according to the documentation, that's why try to remove them.