-
Notifications
You must be signed in to change notification settings - Fork 510
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
Improve tooltip for Elasticsearch tag queries #1809
Conversation
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Codecov ReportPatch has no changes to coverable lines.
📢 Thoughts on this report? Let us know!. |
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@@ -342,6 +342,17 @@ export class SearchFormImpl extends React.PureComponent { | |||
<li> | |||
Values containing whitespace or equal-sign &apos=&apos should be enclosed in quotes | |||
</li> | |||
<li> | |||
Elasticsearch/OpenSearch storage requires escaping{' '} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to belong in the tool tip. The only escaping needed in the ui input is so that the ui parser can pick apart the right strings. If those strings still need additional escaping for storage, it should be handled by the storage implementation, transparently to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ES storage uses on purpose regex terms (it regex support was added to ES storage some time ago).
The goal here is to document this to let people know that escaping is needed for exact match queries.
> | ||
reserved characters | ||
</a>{' '} | ||
need to be escaped for exact match queries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it include an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how to render the example. There is already one example (not for ES). If ES specific examples are added, we should let users know that they are for ES and I am not sure how to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just go with the current PR.
Resolves #1733