Split value production events #406
Conversation
| case ReturnValueProduced returnValueProduced: | ||
| OnReturnValueProduced(returnValueProduced); | ||
| break; | ||
| case ValueUpdated valueUpdated: |
There was a problem hiding this comment.
Does this indicate a return value was updated or also a display data?
There was a problem hiding this comment.
return value maps to ExecuteReply, ma I answering your question?
There was a problem hiding this comment.
Is ValueUpdated being used for both return values and values produced using display?
There was a problem hiding this comment.
display(x) produces a ValueProduced event, ValueUpdated is generated by calling the d.Update(x) method
|
|
||
| private void OnReturnValueProduced(ReturnValueProduced returnValueProduced) | ||
| { | ||
| var openRequest = InFlightRequests.Values.SingleOrDefault(); |
There was a problem hiding this comment.
I've hit a bunch of bugs with this line that was copy-pasted. I changed it to TryGetValue in my PR.
| } | ||
| } | ||
|
|
||
| private void OnValueUpdated(ValueUpdated valueUpdated) |
There was a problem hiding this comment.
There's a lot of repeated code here. Maybe a base type for the various "value produced / updated" type events so they can go through the same method?
| { | ||
| public class ValueProduced : KernelEventBase | ||
| { | ||
| public ValueProduced( |
There was a problem hiding this comment.
Does this now correspond to display data?
|
|
||
| events.Should() | ||
| .ContainSingle(e => e is ValueProduced) | ||
| .ContainSingle(e => e is Events.DisplayedValue) |
There was a problem hiding this comment.
DisplayedValueProduced
There was a problem hiding this comment.
I will get there .. eventually
| namespace Microsoft.DotNet.Interactive.Events | ||
| { | ||
| public class ValueUpdated : ValueProductionEvent | ||
| public class ValueUpdated : ValueProducedEventBase |
There was a problem hiding this comment.
So is this more appropriately called DisplayedValueUpdated?
Emit different events to clarify the semantic
when code submission evaluates to a value -> ReturnValueProduced -> ExecuteReply
when a value is produced (via display or console redirection) -> DisplayedValueProduced-> DisplayData
when a value is updated (via Update api) -> DisplayedValueUpdated-> UpdateDisplayData