-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Implementing line comments #3126
Implementing line comments #3126
Conversation
Adding tests Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
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.
One little nit - I want to make sure we're not parsing valid filters that include #
as comments now.
After that it looks good, but could you add a bit to the docs showing the new support?
{ | ||
in: `#{app="foo"} | json`, | ||
err: ParseError{msg: "syntax error: unexpected $end", line: 1, col: 20}, | ||
}, |
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.
How does {app="foo"} |= "#"
result? Can we add a test for this (I'm not sure if this would be interpreted correctly as a filter expression or incorrectly as a comment.
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.
That's a good idea - I've added some further tests to this end
…xed as a comment Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Done and done good sir 👍 |
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.
LGTM, thanks
What this PR does / why we need it:
This PR implements line comments using the
#
character.Which issue(s) this PR fixes:
Fixes #3112
Special notes for your reviewer:
I have not yet added documentation because it doesn't seem like we publish the LogQL grammar in the docs, and it feels a bit silly to document a line comment 🙃
Checklist