-
Notifications
You must be signed in to change notification settings - Fork 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
Fix precedence of safe-accessor tail #525
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build c293e8ca5c743b40acee1500092d00da4d6541df-PR-525Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Can you add a description of what this is actually doing for posterity? It looks vaguely related to #532 but doesn't fix it. |
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.
The description is good but hard to grok; can you also add a test case that would have failed before but should pass now and vice versa?
We really should fix the stack operator in wbnf so that this rule can be self-referential. arr-ai/wbnf#85 |
Fixes #552
Change safe-accessor tail to expect an expr instead of the next rule on the precedence stack.
The expression
a.b?:c.d
used to parse as(a.b?:c).d
. This is confusing because it means that.d
, which looks like it belongs toc
, actually applies toa.b
ifa
has ab
attribute. This PR changes it to parse asa.b?:(c.d)
.This is actually a bit of a hack. My preference would be to loop back to the current rule. However, wbnf doesn't have a way to refer to the current rule, only the next rule (
@
) or back to the top (expr
). It might be better to add this capability to wbnf. Comments welcome.Checklist: