-
Notifications
You must be signed in to change notification settings - Fork 35
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
Support building infix logic representation #1530
Support building infix logic representation #1530
Conversation
Test summary 4 215 files 6 481 suites 15m 4s ⏱️ Results for commit 5c0b09c. ♻️ This comment has been updated with latest results. |
InfixLogicBuilder
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.
@sethrj I think this is ready now.
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.
Sorry I've had these queued up for a couple days but forgot to send
@sethrj @elliottbiondo Ready for a 2nd look. |
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.
I think this is good enough, we have enough to do besides going in circles about the details of the details of the details here. Thanks!
Refactor the
PostfixLogicBuilder
to support other types of logic notations. In particular, support for infix notation as part of #1282.PostfixLogicBuilder
is now a free function calledbuild_logic
that is templated on a policy object dictating the notation to build. Any state is now held in the policy object instead of the builder object andbuild_logic
provides common behavior (faces mapping).This simple implementation inserts parentheses around every single
Joined
operator even if they wouldn't be necessary. This also doesn't check that the tree is in a valid infix-compatible form (noNegated {Joined{}}
node)