-
Notifications
You must be signed in to change notification settings - Fork 323
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
Editing the node is not changing the placeholders and widgets #7595
Editing the node is not changing the placeholders and widgets #7595
Conversation
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.
nits
@@ -250,6 +250,7 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { | |||
} | |||
} | |||
|
|||
@CompilerDirectives.TruffleBoundary |
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.
Shouldn't this be on setExecuted
instead?
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.
Right
call <- | ||
if (Types.isPanic(value.getType)) Option(value.getCallInfo) | ||
else Option(value.getCallInfo).orElse(Option(value.getCachedCallInfo)) |
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 know that this logic has been here already but it kind of looks weird that first we ask for call info and if missing we are asking for the cached value. I would expect the opposite to happen.
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.
This logic covers the case when the user comes back from the function (pops the execution frame). In this case, the expression does not have the call info because the result was cached and no function was called, but contains a cached call info.
I agree that it's weird but it always breaks some other cases when I try to come up with a workaround.
Pull Request Description
close #7462
Important Notes
enso-7462.mp4
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.