Add context argument to grammar recognizers (pull request) #55
Conversation
- Updated built-in recognizers in grammar.py to accept the new argument. - Added context object to _next_token() and _token_recognition() calls in order to be able to pass it to the recognizers. - Initialized the context attributes earlier in parse() so that this occurs before the first recognizer is called.
…cognizers. Recognizers should now return None if no match was found. Returning an empty string indicates a zero-length match, which could be useful when the recognizers utilize external state set by actions.
Also added initialization for all used context attributes at the beginning of parse().
…ment. - Context argument was removed from built-in recognizers to minimize changes. - Modified Grammar class to use introspection to determine whether recognizers accept a context argument or not. This information is stored as a recognizer attribute to speed up recognizer signature checking during parsing. - Updated parser to check the recognizer signature to determine whether or not to pass the context argument to the recognizer. - The context argument was moved to the beginning of the recognizer argument list.
This function uses inspect.signature if it is available, otherwise falls back to using inspect.getargspec, which is deprecated for Python 3.
- The number of parameters is now checked in the property setter function, which sets the _pg_context attribute appropriately. - Removed Grammar._resolve_context_arg_presence_for_recognizers(), which is no longer necessary.
This seems to work for a simple example, but quite likely is not correct in all cases.
|
Looks good. Merging. Thanks for your contribution. I'm in the process of reworking parser to further improve on error reporting and I'll probably introduce some changes in the way context is handled. I'm thinking to do shallow copy of context as all its elements are immutable, while deep copying of |
|
This optional addition of context to recognizer would need some docs. So if you find some time to do it it would be greatly appreciated :) |
|
It's great to see this merged in! Thanks for being open to my suggestions. I'll try to take a look at the documentation before too long. |
|
It looks like there is a bunch of work going on in the error-reporting-rework branch. Is it okay if I make a couple of documentation changes based on the master branch, or should I wait until things settle down? I don't have time to write a lot of documentation, but I can add a description of the "extra" attribute and copy/deepcopy behavior in the Context object description of the Actions section and mention the optional context argument in the Recognizers section if that would be helpful. |
|
Hi Jeff. Yeah, there is a lot of changes going on at the moment so maybe it's better to wait some time for it to settle down. |
|
No problem. I thought that might be a good idea. |
This pull request is a result of the discussion in #50.
The changes in this branch allow parsers to be extended by storing some extra state information in the context object that is manipulated by custom actions and examined by custom recognizers. In order to achieve this, parglare was modified to allow recognizers to accept a context object as an optional first argument. The change is backward-compatible with existing recognizers. Python's introspection features are used to examine the recognizer's signature in order to determine the number of arguments to pass to the recognizer.
An attempt was made to make this work for GLR parsers by cloning the context object's "extra" attribute that is set aside for storing the extra parsing state. Having user state stored in a special context attribute should be more efficient than cloning the whole context object and also reduces the possibility of name collisions between the context attributes used by parglare and those added by the user. I don't understand all of the GLR logic, so it's likely that I missed something in the cloning and restoring of the context object, but this code does work for the example I posted in #50.