Open connor4312 opened 8 months ago
@thegecko and I had some discussion on the linked issue, continuing here https://github.com/microsoft/vscode/pull/197458#issuecomment-1793666100
I'd love to have better support for visualizations!
I think a fundamental question is if these visualizations should be a modal one-time experience (as in VS https://youtu.be/1Nq4E4aN1WA?t=85, maybe you can see it as copilots "inline chat") or a more interactive non-modal experience (as in my debug visualizer extension, which would correspond to copilots ghost text once you setup the visualization).
The modal experience is much easier to implement, but much more inconvenient to use when you want to inspect how value visualizations change as you step through your code, as the visualization disappears when you step. In particular, the non-modal experience has to remember which visualization you selected the last time for a given variable, so you don't have to select it again the next time you want to visualize the same variable.
I would really like to see the non-modal experience, as it is a much more useful feature.
My initial thought at an API is something like this.
This basically replaces the "generic" tree in the hover with a specialized, extension-contributed one, right?
I think extensions might want to be able to return ProviderResult<TreeView | undefined>
to indicate they cannot visualize the given data (even though it matched the selector
). E.g. a base64 hex viewer is only applicable to strings that match [A-Fa-f0-9=]+
, which you cannot easily express statically through a selector.
It would also be nice if one VisualizationProvider could return multiple possible visualizations (ideally without having to compute the tree yet). Then visualization extensions can delegate listing the visualizations to the program to debug.
Maybe the non-modal approach could work like this (similar to how SuggestProviders work):
namespace debug {
/**
* Registers a custom data visualization for variables when debugging.
*
* @param selector Identifies data this visualizer applies to. It will only
* be shown for DAP variables whose properties are a superset of the
* `variable` property, and whose session type is `debugType`.
*
* @param provideView Invoked for a variable when a user picks the visualizer.
* It may return a {@link TreeView} that's shown in the Debug Console or
* inline in a hover.
*/
export function registerDebugVisualizationProvider<T extends IDebugVisualization>(
selector: { debugType?: string; variable?: Dap.Variable },
// Serves as dynamic discoverability mechanism (selector applies first though).
provide: (context: IDebugVisualizationContext) => ProviderResult<T[] | undefined>,
// Allows to delay computing `treeView`
resolve: (visualization: T) => Promise<void>;
): Disposable;
export interface IDebugVisualization {
name: string; // A human readable (localized) name of this visualization
id: string; // The id to remember which visualization was picked the last time
treeView?: TreeView; // If this visualization is selected, this tree view replaces the "generic" tree view of the value
visualizeCommand?: ICommand; // This command is shown next to the value when an inline visualization cannot be done.
}
export interface IDebugVisualizationContext {
/** The variable the user wants to visualize */
variable: Dap.Variable;
/** The container the variable was set in. */
// authors note: this and the name is required for use of `setVariable` / `setExpression`
container: Dap.Variable;
/** The name of the variable in its container. */
name: string;
/** The debug session the variable belongs to. */
session: DebugSession;
/** The currently selected debug visualization for this value. This gives extensions a chance to use a single DAP request for discovery and visualization computation. In that case, a resolve call wouldn't have to send a second DAP request to actually compute the visualization */
preferredDebugVisualizationId?: string;
}
}
These should not be custom webviews.
I think the super power from visualizations really comes from custom UI. But I understand its a hassle to implement (also, because some visualization libraries are large and take time to load, you want to reuse the webview). ~A nice escape hatch for this could be https://github.com/microsoft/vscode/issues/125763 where extensions could then open the webview on their own when the user clicks on some "visualize" command.~ You already seem to have thought of this.
It may be useful to outline some real-world use cases which this proposal may (or may not!) be designed to support.
Some further thoughts:
@hediet
Persistence: it would be possible for an extension who opens its own webview to also have a DebugAdapterTracker to update a webview as the state changes.
Provider/resolver: I agree, that is a good idea. Updated the API in the main issue. "This basically replaces the "generic" tree in the hover with a specialized, extension-contributed one, right?" -- yes
Webviews: yes, webviews are more of a nightmare when it comes to showing them inside tree views. This is a path trod by notebooks in years past. For performance, showing many individual webviews is a poor experience, so notebooks ended up with the editor layer and webview layer. But getting this right has taken years and is still presenting challenges, so I have no appetite at all to do that here. But, as mentioned, inline 'popouts' might be a way to make this happen, and could be added in the future by adding some extra | WebviewPopout
to the resolve method's return type.
@thegecko
I'll update the main issue with scenarios to support. Searching/filter: I don't think this is in scope for this work.
Other thought. I realized the "variable" selector as I have defined it is not very good, since e.g. the common case thegecko is looking would want any defined memoryReference
.
If we continue to go with an object selector, I would type it as variable: Record<string, boolean | string | RegExp>
with the rules:
{ memoryReference: true, type: /string|buffer/ }
would match any variable that has a defined memoryReference
and set their type
to string
or buffer
I don't want to have a more tailored selector with properties like hasMemoryReference: boolean
to avoid leaking DAP knowledge into vscode.d.ts.
Other thought. I realized the "variable" selector as I have defined it is not very good
We currently use when
clauses in the manifest file to ensure things appear at the right point, I wonder if that system can be leveraged?
It just so happens there exists a canViewMemory
clause in VS Code which is true when the current variable has a memoryReference
.
This should allow the hex editor to move to using the generic APIs and away from the custom handling we currently have.
I would find it particularly nice, if the "visualization extension points" could also be reused not only for the hex editor, but also for "Go to source location" proposed in https://github.com/microsoft/debug-adapter-protocol/issues/372. Afaict, the current proposal would already support adding such a "Go to source location" button, or am I missing something?
also, CC @vadimcn since the proposed extension points here might also be very useful for https://github.com/vadimcn/codelldb and its data visualizations
A visualizer may choose to return
undefined
from
- this function and instead trigger other actions in the UI, such as opening
- a custom {@link WebviewView}. */ resolveDebugVisualization(visualization: T): ProviderResult<TreeView | undefined>;
I think this should align with the resolve
idea/signature of completion providers (I'm very sure Joh will insist on this anyway).
Also, I don't think resolve should cause side effects, such as opening a webview.
This will make it impossible to automatically update the tree view (e.g. in the watch window) when you step through the code.
Rather it should return the command that does this.
WebviewPopout
That would be nice!
We currently use when clauses in the manifest file to ensure things appear at the right point, I wonder if that system can be leveraged?
Good point, we should do this for extension activation anyway.
I think this should align with the resolve idea/signature of completion providers (I'm very sure Joh will insist on this anyway)... Also, I don't think resolve should cause side effects, such as opening a webview.
Maybe, we do have precedent for resolve*
causing side-effects, such as resolveWebviewView
. Will discuss it.
I've done some initial work on this, as well as initial refinement in the API, which is in the main branch of VS Code. It currently supports command-generating visualizers. You can see an example of a basic 'base64 visualizer' in https://github.com/microsoft/vscode-extension-samples/tree/connor4312/debug-viz-demo/proposed-api-sample, and try it in VS Code Insiders.
@connor4312 nice work! I think this is an excellent addition and looks very flexible.
(obviously not base64 but modified to get the viz to show)
Are there any specific things you want feedback or testing around?
Any feedback you want to provide on the initial version is great -- though the current exposure is fairly simple and "works for me" is also good feedback 🙂
The more gnarly part will be letting it support subtrees, which I'll look at this coming iteration.
This sounds interesting. I wonder how it'll affect or help with C++ things (Natvis and GDB pretty printers, which I think the MS C/C++ extension currently supports).
Any feedback you want to provide on the initial version is great -- though the current exposure is fairly simple and "works for me" is also good feedback 🙂
Well, it "works for me"! As long as the hex edit recommendation is removed when this is merged :)
Playing some more, I notice multiple visualizations are put into a menu:
Would it be worth supporting group names (including inline) to control this and group accordingly if multiple appear?
The more gnarly part will be letting it support subtrees, which I'll look at this coming iteration.
Is this supporting branches further down the graph?
Hi @connor4312 , since this is a big work item, I suppose this will not be finalized this milestone, hence I am changing the milestone to March 2024
. Feel free to change it back if you'd like to make this a candidate issue.
Just tried out the API again.
What when
context should I use when I can visualize everything? "true" did not work.
Also, it seems like it only works in the variable view, not in the watch view or with hovers. Would be very nice if this would be supported there as well.
@connor4312 can we have an update on this being merged, please? We are keen to promote our new memory inspector in the VS Code UI: https://marketplace.visualstudio.com/items?itemName=eclipse-cdt.memory-inspector
VS proper has some data visualization like this:
...and personally I've never been a fan of how we represent things like XML elements or binary data in debug hovers and the REPL. In large part this is because we have to have a 'generic' view that gives all possible information, so we can't be more concise when showing things like DOM tree.
So it would be nice to have a way for visualizations to be contributed, with similar affordances to what VS provides.
My initial thought at an API is something like this. Note that we don't bring in DAP types in vscode.d.ts, so they would be typed as
unknown
and cast by the extension author.cc @hediet @roblourens @joj