-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
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