-
Notifications
You must be signed in to change notification settings - Fork 3
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
Include (Python) listener #79
Conversation
@svengiegerich just a small suggestion, keep the changes in the generating code from the changes in the generated code in different commits. In this case is not critical, but when there are multiple manual changes, particularly big ones, it's hard to distinguish from the autogenerated code. |
Thanks! You're right, I was lazy this time 🙈 |
(In light of PR #73, I'll need to update this PR before the review can continue) |
4478021
to
4859649
Compare
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.
Could consider adding a test that asserts one of the "listener" callbacks is called when parsing a simple SQL statement.
Thanks, @SimeonStoykovQC.
I do like the idea! But how would I do this? Do I need to mock the class |
You can try, yes. Do you have an idea how you will use it -- from what I see it's just method stubs with |
In the end - outside of the package - I'd like to do smth like this:
|
Merging now; I'll add the test separately. |
Motivation: I'd like to use our grammar's (Python) listener to facilitate the parser's utility beyond SQL execution.
More concretely, I'm currently playing around with extracting a dependency tree for my SQL-based pipeline based on the listener.
👉 I propose to ship the listener within the package
Changes: Removed arg
no-listener
for Python target & re-generated grammar