-
Notifications
You must be signed in to change notification settings - Fork 4
Introduce a PylasuANTLRParser class modeled after Kolasu. #42
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
Conversation
…rything was copied from Kolasu, just the bare minimum to have this usable in a project.
ftomassetti
left a comment
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.
I just added minor comments
| for field in fields: | ||
| if field.name == name and PYLASU_FEATURE in field.metadata: | ||
| feature = field.metadata[PYLASU_FEATURE] | ||
| for fld in fields: |
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.
Why renaming field into fld?
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.
It wasn't passing the linter because it shadowed an import
| @@ -1,0 +1 @@ | |||
| from .results import FirstStageParsingResult, ParsingResultWithFirstStage | |||
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.
Nice touch!
| You should extend this class to implement the parts that are specific to your language. | ||
| Note: instances of this class are thread-safe and they're meant to be reused. Do not create a new PylasuANTLRParser |
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.
Does this come verbatim from Kolasu or is this class really thread-safe?
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.
It comes from Kolasu but I don't see why it shouldn't apply here too
| position=Position(token_end_point(last_token), token_end_point(last_token)) | ||
| ) | ||
| ) | ||
| # TODO Kolasu also traverses the parse tree searching for exceptions |
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.
Is this something we should do here?
Should we add a test for this? Or perhaps add an issue to come back to this later?
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.
I'll add an issue
|
|
||
|
|
||
| @dataclass | ||
| class LexingResult(WithIssues): |
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.
Don't we need also the LexingResult, in case just lexing is invoked?
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.
We'll need it but it has a different structure in Kolasu, at the moment it's not used (because we don't have the lex methods). I'll open an issue about that too
Not everything was copied from Kolasu, just the bare minimum to have this usable in a project.
Fixes #18