-
Notifications
You must be signed in to change notification settings - Fork 172
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
Trailing comma in label sets #251
Comments
This is on purpose, to keep the grammar as small as we can. |
Doesn't it just amount to doing this?
That doesn't seem to make it very much bigger? Or do you mean "small" as in the implementation? I can't imagine this would make it very much more difficult. Not sure if there's anything to do about the breaking change though. |
That's a larger grammar, so more work for all parsers. If every little
deviance was allowed for then you end up with a far more complex parser,
which is an issue with the Prometheus text format. Accordibgly the OM
grammar is strict at little to no cost to exposition. Asking for an extra
line of code in exposition isn't much.
…On Fri 29 Jul 2022, 11:53 Victor Nordam Suadicani, ***@***.***> wrote:
Doesn't it just amount to doing this?
labels = "{" [label *(COMMA label) [COMMA]] "}"
That doesn't seem to make it very much bigger? Or do you mean "small" as
in the implementation? I can't imagine this would make it very much more
difficult.
Not sure if there's anything to do about the breaking change though.
—
Reply to this email directly, view it on GitHub
<#251 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABWJG5TOWNM5AYGKKFA4GTTVWOZZZANCNFSM55AAQ2CA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Prometheus' docs on the exposition format quite clearly allow for a trailing comma in label sets.
However, the exposition format in the spec doesn't seem to allow this.
I think trailing commas are fine and should be allowed. It makes it easier to generate the label set as you can just put a comma after each label without having to worry if there's more labels.
The text was updated successfully, but these errors were encountered: