-
Notifications
You must be signed in to change notification settings - Fork 324
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
Port graph editor to new AST #4113
Conversation
b83f430
to
740d9c1
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.
I don't have words to tell you how happy I am it's here. Before merge please:
- Be sure someone did Q/A of it. Ask about it on Discord.
- Add doc comments where necessary. There are some code pieces not commented at all.
@JaroslavTulach or @hubertp -- could you review the backend change? The parser now rejects import statements that have |
Keziah Wesley reports a new STANDUP for yesterday (2023-02-07): Progress: Removed Next Day: Next day I will be working on the #4113 task. Implement any review changes. Work on the remaining high-priority follow-up issue (#5571). |
Keziah Wesley reports a new STANDUP for today (2023-02-08): Progress: Implemented review changes (docs); fixed issues I found in additional testing. Next Day: Start working on performance. |
@JaroslavTulach @hubertp please review the engine changes in the PR - it would be great if we could land it ASAP ant test the effects of replacing parser in IDE |
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.
Engine change LGTM
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.
QA acceptance passed for 197ddb13fb698a68f9061322735a66b1efeb8bbb
and Engine version 2023.1.1-nightly.2023.2.9
. No issues related to this PR were were found though I found a couple of issues on develop
.
Pull Request Description
Use the Rust parser rather than the Scala parser to parse Enso code in the IDE.
Implements:
There is additional functionality needed before the transition is fully-completed, however I think it's time for this to see review and testing, so I've opened separate issues. In rough order of urgency (these issues are also linked from the corresponding disabled tests):
case-of
and partial macro matches #5571SKIP
andFREEZE
to new AST #5572graph_controller_connections_listing
for new parser #5574Important Notes
The implementation is based partly on translation, and partly on new analysis. Method- and operator-related shapes are translated to the old
Ast
variants, so that all the analysis applied to them doesn't need to be ported at this time. Everything else (mostly "macros" in the old AST) is implemented with new analysis.Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide build
and./run ide watch
.