-
Notifications
You must be signed in to change notification settings - Fork 8
fix: compare with 'in' operator #15
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
Conversation
cc @tjdevries |
grammar.js
Outdated
//"[", $.expr, "]", | ||
//":", | ||
//$.expr, | ||
$.field, |
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 is not entirely according to spec but it prevents a precedence issue for fieldnames in brackets { [fieldname]: value }
. I found this when making PR #16 but solved by this change.
test/corpus/fieldname.txt
Outdated
============================= | ||
Resolve expr in fieldname key | ||
============================= |
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.
Test case from #16
{ | ||
['item' + i](v): i + v | ||
for i in std.range(0,10) | ||
} |
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 isn't possible according to the specification but is correctly intepreted by jsonnet.
// a.jsonnet
local lib = {
['item' + i](v): i + v
for i in std.range(0, 10)
};
lib.item10(3)
[I] ➜ jsonnet a.jsonnet
13
[I] ➜ cjsonnet a.jsonnet
13
[I] ➜ jrsonnet a.jsonnet
13
After you rebase, i'll take a look at this one as well. Sorry for the delay! |
4707122
to
9f9d0c6
Compare
rebased |
@tjdevries let me know if there's something I can improve |
LGTM to me! Thanks! |
Binary op "in" comparison
'string' in obj
wasn't working properly, this PR adds it to the grammar.This PR also adds test for comparison and for loops as they both deal with the 'in' keyword.
Also fixes #13 (see comments)