Engine should send notification about node status #3729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaroslavTulach
commented
Sep 22, 2022
JaroslavTulach
commented
Sep 26, 2022
...rument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
Show resolved
Hide resolved
JaroslavTulach
commented
Sep 26, 2022
engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala
Outdated
Show resolved
Hide resolved
JaroslavTulach
force-pushed
the
wip/jtulach/NodeStatusNotifications_182818735
branch
from
285dd16
to
2c7652d
Compare
JaroslavTulach
requested review from
MichaelMauderer,
wdanilo and
mwu-tow
as code owners
|
I still need to write some unit tests for the new message, but otherwise I believe this PR is ready for review. Thanks in advance for your opinions. |
farmaazon
approved these changes
Sep 27, 2022
JaroslavTulach
commented
Sep 27, 2022
...with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala
Show resolved
Hide resolved
4e6
reviewed
Sep 27, 2022
There was a problem hiding this comment.
Looks ok. Things that can be improved here:
- sending message with a list of pending IDs would be more efficient, than a bunch of messages for each ID
- The
typeandmethodCallof the newExpressionUpdatemessage are always null (another reason for sending pending IDs in a batch). It should be at least documented in the protocol description
4e6
approved these changes
Sep 27, 2022
JaroslavTulach
commented
Sep 27, 2022
JaroslavTulach
added
the
CI: Ready to merge
mergify
bot
deleted the
wip/jtulach/NodeStatusNotifications_182818735
branch
2 tasks
2 tasks
hubertp
added a commit
that referenced
this pull request
May 18, 2023
The change adds an additional field to `ExpressionUpdates` messages sent by `ProgramExecutionSupport` to indicate if the type of value (or its method pointer) has changed and therefore would potentially require a suggestions' update. Prior to #3729 that check was done during the instrumentation. However we still want to continue to support "pending expression" functionality therefore `SuggestionsHandler` will use the additional information to filter only the required expression updates. Most of the changes are related to adapting our tests to the new field. Closes #6707.
4 tasks
mergify bot
pushed a commit
that referenced
this pull request
May 18, 2023
The change adds an additional field to `ExpressionUpdates` messages sent by `ProgramExecutionSupport` to indicate if the type of value (or its method pointer) has changed and therefore would potentially require a suggestions' update. Prior to #3729 that check was done during the instrumentation. However we still want to continue to support "pending expression" functionality therefore `SuggestionsHandler` will use the additional information to filter only the required expression updates. Most of the changes are related to adapting our tests to the new field. Closes #6706. # Important Notes The associated project now loads and navigates smoothly. Also attaching a screenshot from the project that illustrates that pending functionality continues to work: [Kazam_screencast_00006.webm](https://github.com/enso-org/enso/assets/292128/35918841-f84f-4e1c-b1b0-40e45d97e111)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
When nodes get invalidated in the cache, they have to be recomputed. Let the IDE know which of the nodes are pending by sending
Api.ExpressionUpdate.Payload.Pendingmessage.Important Notes
This PR introduces new
Api.ExpressionUpdate.Payload.Pendingmessage. This message is delivered before re-computation of nodes. LaterApi.ExpressionUpdate.Payload.Valueor other is sent to notify the IDE that a value for given node is available.Trivial implementation of of the
Api.ExpressionUpdate.Payload.Pendingmessage in the IDE is provided by this PR to (improperly) visualize pending node status - further improvements needed in follow up PRs.Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide buildand./run ide watch.