Make data viewer openable from the variables window while debugging#14269
Make data viewer openable from the variables window while debugging#14269joyceerhl wants to merge 9 commits intomicrosoft:mainfrom joyceerhl:tensor-variable-context-menu
Conversation
joyceerhl
requested review from
DavidKutu,
DonJayamanne,
IanMatthewHuff,
greazer,
jmew and
rchiodo
joyceerhl
changed the title
(TODO find a better place to add the __vscodeVariableMenuContext property to the DebugProtocol.Variable)
| } | ||
| return true; | ||
| }); | ||
| const allowedVariables = variablesResponse.body.variables |
There was a problem hiding this comment.
We might want to get in touch with Debugger team about this.
Its something they might find useful or be aware of (the fact that we're adding this).
Personally I'd prefer if all of this was done in debugpy and we just hooked into some message or the like (e.g. we could use capabilities or other flags & use custom messages)
@int19h @karthiknadig /cc
There was a problem hiding this comment.
It would be hard to make it quite as flexible as client-side filtering. If we hardcode the filters, debugpy would have to be touched every time this logic needs revising. And if we don't hardcode them, then the protocol would need to be expressive enough to express all those conditions, which would also make the implementation expensive.
I think client-side filtering is the better option here, unless there are important perf reasons to exclude some variables.
There was a problem hiding this comment.
Just in the interest of clarity (even though I think the conclusion is the same), we're not asking for filtering logic in debugpy. What we're after is debugpy setting the __vscodeVariableMenuContext property on DebugProtocol.Variables before they are sent over to VS Code, several lines down from this spot: https://github.com/microsoft/vscode-python/pull/14269/files#diff-d8fc76e8c5b19f7c5df2b4282409e727R297-R300
As background for anyone unfamiliar with why we need this, the reason we need this property is so that we can make use of the new contribution point for the variable context menu introduced in the August release. VS Code uses the hidden __vscodeVariableMenuContext property to set a per-variable debugProtocolVariableMenuContext key with the same contents (details). Extensions can then use the debugProtocolVariableMenuContext key in when clauses that specify whether a given context menu item with an associated command should be displayed for a given variable.
joyceerhl
changed the title
| } | ||
| } | ||
|
|
||
| private async onVariablePanelShowDataViewerRequest(request: IShowDataViewerFromVariablePanel) { |
There was a problem hiding this comment.
This seems like a weird spot to handle this. What if there isn't an interactive window open? Seems like it should work without a notebook.
There was a problem hiding this comment.
Yeah @jmew clarified with me this morning that we want this to work for regular debugging too, so this will need reworking (currently only works when debugging in the IW)
| } | ||
| } | ||
|
|
||
| // This special DebugAdapterTracker function listens to messages sent from VSCode to the debug adapter |
There was a problem hiding this comment.
I think you should move this into the DebugLocationTracker. It really does this too (and seems like it's also incorrect). Then make that DebugLocationTracker some global service that this code could use.
|
Kudos, SonarCloud Quality Gate passed!
|
|
Was able to test that this works with debugging Python files through the 'attach' config (debugging via 'launch' is broken for me ATM for some reason). Please review when you have the time! |
|
Actually, going to open a PR off of the main repo. AzDO PR validation still isn't being reported on PRs from forks |
6 participants
Per @greazer, changing the scope of this PR to provide users with a way to open the data viewer for data-viewable types, i.e.:
TODO:
jupyterVariables.getMatchingVariablewhich is not implemented for oldJupyterVariables