-
Notifications
You must be signed in to change notification settings - Fork 1
Add advanced setting for disabling leading wildcards in kuery #14
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { KbnError } from '../../errors'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This is the first KbnError that's not just in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the difference here is that these errors are only meant to be thrown by the parser, so it makes sense to co-locate them with the grammar. Eventually I imagine us having a bunch of these parser errors. I wouldn't have even extended In general I've been trying to keep all kuery code inside the kuery directory so it doesn't become an incomprehensible mess spread across the entire codebase (see: visualize). |
||
|
|
||
| export class NoLeadingWildcardsError extends KbnError { | ||
| constructor() { | ||
| super( | ||
| 'Leading wildcards are disabled. See query:allowLeadingWildcards in Advanced Settings.', | ||
| NoLeadingWildcardsError | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,3 +33,8 @@ export function toQueryStringQuery(node) { | |
| const { value } = node; | ||
| return value.split(wildcardSymbol).map(escapeQueryString).join('*'); | ||
| } | ||
|
|
||
| export function hasLeadingWildcard(node) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This seems to be the first function not aimed at building or converting a node, but testing a value. Maybe it makes sense to do this in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test function is also not aimed at building or converting, so I think there's precedence here. Also, these objects aren't strictly immutable, so the node could change and then |
||
| const { value } = node; | ||
| return value.startsWith(wildcardSymbol); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
Is the convention here that
query:queryStringwill be for lucene settings, whilequery:<anything else>will be for Kuery?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.
Yeah I suppose so, hopefully we won't be adding any more settings specifically for lucene and eventually we'll remove
query:queryString:options