-
Notifications
You must be signed in to change notification settings - Fork 11
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
Remove unneeded Juxt
token
#208
Conversation
scrapscript.py
Outdated
@@ -394,8 +388,6 @@ def parse(tokens: typing.List[Token], p: float = 0) -> "Object": | |||
# we can match variants in MatchFunction | |||
# It needs to be higher than the precedence of the && operator so that | |||
# we can use #true() and #false() in boolean expressions | |||
# It needs to be higher than the precedence of juxtaposition so that |
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.
Please leave this comment; still applies
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.
Oh, I see. Thanks for pointing that out. I've restored this hunk.
Thanks for the fix! Please add back the comment or I'll do it in a couple of days. Then we merge! If you would like to work on anything else, please let me know |
I saw that #84 was about utilizing Pratt parsing, but it seems like you've already implemented a pretty neat Pratt parser for the language. Are there any other parts of the parsing code that could use some help? A couple of things I was thinking about:
|
84 is mostly about deciding if we want to have the lexer be a generator and have the parser pull from it. But I also dropped a Pratt parsing resource because our parser is a big ball of hacks and I wonder if it's possible to make it more regular-looking.
That is a good question. I think it feels too low in the precedence list but didn't have any actual notion of what that means or why it's wrong. Added in 33a6c8f but not explained well. |
Thank you for the PR! |
I started reading through the parsing code and noticed that this token was being unused. Let me know if you'd like me to make any further changes!