-
Notifications
You must be signed in to change notification settings - Fork 14.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: add support for comments in adhoc clauses #19248
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19248 +/- ##
=======================================
Coverage 66.78% 66.79%
=======================================
Files 1670 1670
Lines 64401 64425 +24
Branches 6501 6502 +1
=======================================
+ Hits 43009 43030 +21
- Misses 19708 19711 +3
Partials 1684 1684
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 looks great!
@@ -23,6 +23,14 @@ import { QueryObjectFilterClause } from './types/Query'; | |||
import { isSimpleAdhocFilter } from './types/Filter'; | |||
import convertFilter from './convertFilter'; | |||
|
|||
function sanitizeClause(clause: string): string { |
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 needs to be done on backend, no?
if open_parens > 0: | ||
raise QueryClauseValidationException("Unclosed parenthesis in filter clause") | ||
|
||
if previous_token and previous_token.ttype in Comment: | ||
if previous_token.value[-1] != "\n": | ||
clause = f"{clause}\n" |
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.
Ah, I see it is done in the backend. But I still don't understand why we also do this in the frontend?
* feat: add support for comments in adhoc clauses * sanitize remaining freeform clauses * sanitize adhoc having in frontend * address review comment (cherry picked from commit f341025)
🏷️ preset:2022.11 |
* feat: add support for comments in adhoc clauses * sanitize remaining freeform clauses * sanitize adhoc having in frontend * address review comment
* feat: add support for comments in adhoc clauses * sanitize remaining freeform clauses * sanitize adhoc having in frontend * address review comment
* feat: add support for comments in adhoc clauses * sanitize remaining freeform clauses * sanitize adhoc having in frontend * address review comment (cherry picked from commit f341025)
SUMMARY
Add support for comments in adhoc clauses to make it easier to comment out sections of adhoc metrics/filters when developing charts. This is done by making sure that all freeform queries containing a single line comment
--
are suffixed with a line change.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION