- Notifications
You must be signed in to change notification settings - Fork32
Add support for side-effect actions (pull request)#56
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
… method.- Added _resolve_side_effects() method to Grammar class, which mimics _resolve_actions().- Modified constructor and _init_grammar() to pass along side_effects argument to _resolve_side_effects().- Added call to side_effect action in _call_shift_action and _call_reduce_action Parser methods. Refactored some code out of _call_reduce_action to reduce code duplication between actions and side_effects.- No support for <grammar>_side_effects.py yet.
coveralls commentedAug 1, 2018
coveralls commentedAug 1, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
How does this implementation look? It seems to be working for me. Please let me know if this in on the right track. Thanks. |
I haven't had a change for a deeper look into it. I'll get to that probably after finishing current rework around error reporting. The thing that I don't like with the current side-effect implementation is that a lot of code is copy-pasted from plain actions. I would like to integrate/generalize that so it would be easier to maintain. I could provide more info when get back to it in the next week or two probably. |
Yes, I agree that the duplicated code should be minimized. Thanks. |
This pull request was spawned as a result of the discussion in#51.
This branch divides grammar actions into two groups, based on whether their purpose is to 1) construct a parse result (e.g. AST tree nodes) or 2) manipulate external state for use by recognizers or dynamic filters. If the purpose is manipulate external state, then the action is a side-effect action, otherwise it is a standard action.
A new keyword argument "side_actions" was added to the Parser constructor. Support for a _side_actions.py file was also added to grammar.py.
The option added in#45 was removed, as it is no longer necessary, since side_actions are processed regardless of whether build_tree is true or false.