-
Notifications
You must be signed in to change notification settings - Fork 90
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
Implement curried dot operator #1578
Conversation
core/src/parser/grammar.lalrpop
Outdated
@@ -673,6 +673,9 @@ MatchCase: MatchCase = { | |||
|
|||
// Infix operators by precedence levels. Lowest levels take precedence over | |||
// highest ones. | |||
InfixBOp1: BinaryOp = { |
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.
InfixBOp1: BinaryOp = { | |
InfixBOpAccess: BinaryOp = { |
A minor nitpick, but the other InfixBOp
productions are named according to their operator precedence in InfixExpr
. In fact, I'm tempted to say, that it would be better to put a case like
<l: @L> "." <r: @R> =>
InfixOp::from(BinaryOp::DynAccess())
.eta_expand(mk_pos(src_id, l, r)),
into the CurriedOp
production, because .
is never parsed as a generic infix operator. It rather has its own parsing rules as part of RecordOperationChain
.
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.
Makes sense. Moved implementation inside CurriedOp
.
move implementation inside CurriedOp
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.
Thanks for taking the initiative to implement that. The PR looks good to me 👍
* display icon for nickel file in vscode explorer * implemented with DynAccess. * added tests. * Update grammar.lalrpop move implementation inside CurriedOp --------- Co-authored-by: Ben Yang <ben@ya.ng>
per #1568 , so that
(.) field record
works. It is very pleasant to see that all it takes is just adding four lines of code, and everything works, including the language server. Thank you @yannham & @vkleen for the kind help.