Initial built-in variables view #198468
Initial built-in variables view #198468
Conversation
amunger
force-pushed
the
aamunger/variablesView
branch
from
d5d2456
to
18a0104
Compare
| @@ -207,6 +208,14 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { | |||
| data.supportsInterrupt = Boolean(value); | |||
| _update(); | |||
| }, | |||
| set variableProvider(value) { | |||
There was a problem hiding this comment.
we may want to validate proposed api usage.
| if (token.isCancellationRequested) { | ||
| return; | ||
| } | ||
| this._proxy.$receiveVariable(requestId, variable); |
There was a problem hiding this comment.
If you're just going to pull all the variables here, then it seems like you could just return the variables from this call directly rather than send them over one by one?
There was a problem hiding this comment.
This could stream them into the renderer process asynchronously.
Would there still a purpose to using an asyncIterable in the API if they just are gathered up at once in the EH anyway?
There was a problem hiding this comment.
I suppose it would make sense to send them all at once if I'm going to stick with the non-async tree type - I wasn't really settled on that yet.
There was a problem hiding this comment.
I figured that getting the next variable from the call wouldn't typically be slow, so if there are 10 variables they are going to basically appear all at once anyway, but that you could implement something like paging with this, eg you pull the first 10 variables out of here, display those, and pull more out later if needed. And also for the large array paging thing. This is fine for now of course.
| readonly value: string; | ||
| } | ||
|
|
||
| export class NotebookVariablesTree extends WorkbenchObjectTree<INotebookVariableElement> { } |
There was a problem hiding this comment.
No reason to subclass it, I think
There was a problem hiding this comment.
oh yeah, made more sense for the Async one with three arguments.
|
I'll make some follow-up changes, just so I can make sure to get the bulk of this in |
for #166296
Base implementation of displaying variables from a
VariableProvideron theNotebookController. Only displays the root variables at this point, and doesn't listen to the update event (it just pulls for every execution end event)