-
-
Notifications
You must be signed in to change notification settings - Fork 417
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
Added match stmt support to python.lark #1016
Conversation
Do you have a complicated example of using match? It would be nice to add some tests now that the Python grammar is "official". |
@@ -84,5 +84,4 @@ def test_earley_equals_lalr(): | |||
if __name__ == '__main__': | |||
test_python_lib() | |||
# test_earley_equals_lalr() | |||
# python_parser3.parse(_read(sys.argv[1]) + '\n') | |||
|
|||
# python_parser3.parse(_read(sys.argv[1]) + '\n') |
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.
# python_parser3.parse(_read(sys.argv[1]) + '\n') | |
# python_parser3.parse(_read(sys.argv[1]) + '\n') | |
May I ask what the status of this PR is? It's relevant to a project I'm working on, so if other people don't have time for it I might be able to take it on myself. |
Hi @joseph-e-k , the only thing holding this PR is that there aren't any tests for it. In general, we don't have many tests for the Python grammar yet, but I do run it on the standard library to validate. But the |
@erezsh, I have written some tests for the "match" statement (which pass, you'll be pleased to hear). Could you walk me through the process of actually contributing to this repo? The technicalities tend to be slightly different for each project. So far I have cloned this repository locally, checked out this feature branch (MegaIng/match-python3), merged from master, and added the tests. At this point, which of the following is appropriate?
|
@joseph-e-k I would suggest making a fork and opening a new PR to here (lark-parser/lark) . We can then either merge both of the PRs or just yours. |
Following from #1015