Support global evaluation that requires a file for context

[DanTup]

Currently when evaluating expressions but not stopped at a breakpoint, no context is provided to the debug adapter. In the case of Dart, global evaluation can only be run in the context of a library/file.

It would be useful to have a way for the user to select a file, source, or perhaps some arbitrary context from a set provided by the DA that can be sent with global evaluation requests.

@isidorn @weinand @jacob314

[weinand]

@DanTup before trying to make a proposal for this feature, I need a better understanding of the envisioned scenarios that you want to address.

I tried to make VS Code call the DAP evaluate request without a frameId, but the only place where this seems to be possible is the debug console (when the debuggee is not stopped and consequently no frameId is available).

What do you think should happen in this case? Allowing the user to select a file (to provide the eval context) would be possible, but sounds a bit specific. Above you mentioned that a "library" could be a context too. Allowing the user to select a library is even more specific to Dart.
In theory you could deal with the "global eval" in your extension: if you detect a "global eval" (missing frameId) you could show a Dart-specific picker to let the user select a file or library and pass this as a "private" property of the evaluate request to your DA.

Just adding an optional Source property to DAP's evaluate request would standardise how to communicate a file or library to the DA, but it would not solve the client's problem of what source to pass here.

Are you aware of more scenarios where VS Code (or any client in general) uses a "global eval"?

[DanTup]

I think @jacob314's original idea was that the DA could provide a list of arbitrary contexts that the user could select from (although I think generally a file is the most likely one).

In theory you could deal with the "global eval" in your extension: if you detect a "global eval" (missing frameId) you could show a Dart-specific picker to let the user select a file or library and pass this as a "private" property of the evaluate request to your DA.

I suspect that would probably work fine, and we'd probably just grab the active editor and use that (although it's not completely obvious to the user these things are linked, I think it would generally behave as they are just expecting).

However, I'm not aware of a way we can use middleware to alter this request - is it possible? If not, I think supporting middleware for DAP would be a good generic solution to this without getting into the specifics of Dart.

Are you aware of more scenarios where VS Code (or any client in general) uses a "global eval"?

I'm not - it was specifically the debug console users want to use here. For something like a Flutter app, the app is long-running and the user may work without execution pausing (for example they may make changes to the widgets in their code and hot-reload-on-save). They might want to evaluate some expressions in the context of the file they're editing without needing to hit a breakpoint or something (of course, they will be limited in what they can evaluate).

[weinand]

@DanTup you asked:

I'm not aware of a way we can use middleware to alter this request - is it possible?

Yes, in your extension you could register a DebugAdapterTracker for "dart" debug sessions in order to find the evaluate request with the missing frameId. Then you can add a private "context" attribute to the request which your debug adapter can use when evaluating.

[weinand]

If we do not want to take the DebugAdapterTracker route, we have two options:

1. provide new debug extension API, e.g. an EvaluationContextProvider which does basically the same as a DebugAdapterTracker but with a simpler and typed API. In addition the DAP evaluate request needs a new context attribute for passing the context received from the EvaluationContextProvider to the debug adapter. The type of the context attribute needs to be determined, but most likely it will be the DAP Source type.
2. Again a new attribute context is added to the evaluate request. But instead of receiving the context value from an EvaluationContextProvider, the client (e.g. VS Code), will pass the resource of the active editor as a DAP Source type.

I prefer option 2 because I think option 1 is a bit of an overkill for a rare use case for which the DebugAdapterTracker solution already exists anyway. Furthermore, option 2 continues a pragmatic path we have already taken some time ago: the REPL ties its syntax coloring to the current active editor (@isidorn this is correct, right?).

If option 2 is not sufficient, we can easily add the EvaluationContextProvider API at a later time.

@DanTup @jacob314 what do you think?

[DanTup]

@weinand this all sounds reasonable to me. Could/would the source file being passed in context be a field of the context object to allow for extending it in future if more needs to be added?

Looking at Dart's VM service docs again, I see we may also need to supply an isolate (thread) for the evaluation. I think we can just infer this for most cases (many apps probably only have one isolate), though might be something to consider allowing the user some control over in future. I'll chat with @jacob314 to make sure I fully understand how this works though (he's OOO atm, so won't be imminent) - though I don't think it's required to do the above.

[isidorn]

@weinand correct, the REPL Input and the Breakpoint widget input tie their syntax colouring to the active editor. The REPL tree does not use syntax colouring, thus the tree colouring itself does not depend on the active editor, only the REPL input.

[weinand]

@DanTup I don't think that it is necessary to introduce an additional "context" object because the EvaluateArguments object already has that role: today it contains the frameId, and with my proposal we would just add an activeSource. I do not see any other context value that VS Code could pass generically to the evaluation request.

If Dart would need additional flexibility to pass an isolate (thread) for the evaluation, then this thread cannot be passed by the client (VS Code). It can only come from a Dart specific extension. For this we would need the EvaluationContextProvider and when adding this API, we could easily pass a customContext object.

[DanTup]

I don't think that it is necessary to introduce an additional "context" object because the EvaluateArguments object already has that role: today it contains the frameId, and with my proposal we would just add an activeSource.

Ah yes sorry, I had misread that you were adding the source as a context field. The above sounds perfect! :-)

If Dart would need additional flexibility to pass an isolate (thread) for the evaluation, then this thread cannot be passed by the client (VS Code).

Understood. I think for now I could just handle it in the debug adapter (the original DAP does this, just picking the first isolate). If we need to allow selecting it we might need to come up with something else. I think Jacob's original idea might have been that the DA could provide some arbitrary list that could be shown next to the evaluation input box (eg. Dart could populate it with isolates, but the implementation could be generic) that would be passed back. I'm not sure how often there will be more than one relevant isolate though, so it's probably worth getting his input before trying to come up with a solution for that.

[weinand]

@DanTup thanks a lot for your feedback.

Since the endgame already starts next week, I suggest we finalize this proposal in the next milestone after getting input from @jacob314.

[weinand]

@jacob314 it would be great if you could provide feedback for this proposal so that we can finalise this in January.

[DanTup]

@weinand Jacob is currently OOO (and I believe will be for the rest of Dec)

[DanTup]

@weinand I discussed with Jacob, and it sounds like selecting the thread in the DA will work for most common cases, but won't cover them all. It would definitely be better to handle those cases we can using the proposal above than not, so if supporting the remaining cases is more complex, I'd vote for doing the above initially.

To support all cases correctly, we do need a threadId. If that's something other languages may also use for global evaluation, perhaps VS code could specifically allow selecting a thread? If not, allowing us to provide some arbitrary list (which in our case would be the list of threads) that the user can select would also work - although I'm not sure how this could be presented nicely in the UI.

[weinand]

@DanTup thanks for the feedback - I'll continue with this request tomorrow.

[weinand]

@DanTup @jacob314 how exactly would the user experience with a thread (or context) picker look like?

E.g. like the following?

• we assume that DAP spec has a new getEvaluationContexts request that returns a list of context items (e.g. label, description, ID) and the DART DA implements the request by returning each thread as a context item.
• user types expression into debug console when not being stopped (so there is no frameid)
• since the Dart DA implements supports the getEvaluationContexts request, VS Code pops up a QuickPick that shows all contexts and let the user pick one.
• VS Code passes the picked context to the subsequent evaluate request.

Pros and cons of this approach:

• (+) self-contained within DAP: all contexts used in evaluate request are available through getEvaluationContexts.
• (-) VS Code has to implement a generic Quickpick UI based on data returned from getEvaluationContexts. This will result in a limited UI. E.g. if contexts are hierarchical (e.g. files), VS Code cannot display them as such.

An alternative approach is to let the extension implement the context picker (instead of the DA).

• let's assume an extension API EvaluateContextProvider exists which the DART extension would implement by presenting a QuickPick UI that lets the user choose a thread.
• user types expression into debug console when not being stopped (so there is no frameid)
• VS Code calls the registered EvaluateContextProvider to determine an evaluate context that is then passed to the subsequent evaluate request.

Pros and cons of this approach:

• (+) extension has full flexibility to implement whatever UI is needed to let user pick a context.
• (+) extension can remember the chosen value with any strategy.
• (-) not self-contained within DAP: all contexts passed to evaluate are not available via DAP protocol. This makes it impossible to use the same DA impl with other clients because the picker is implemented outside of DAP.

My minimalistic proposal:

• VS Code just passes the currently open file as a new Source attribute to the evaluate request (and DA determines a thread behind the scenes automatically without prompting the user).

[DanTup]

@weinand that sounds reasonable to us for now. Initially we'll just infer the isolate (as the current legacy DAP does) and then later extend it with a pick-list or something.

It definitely would be nice to have something in the DAP spec rather than custom extension code (since we're trying to make the DAP more reusable by other editors), but perhaps it would be better to discuss that more when we have a working implementation via the extension (and also can see whether there are other languages that might have similar needs to try and keep things fairly general).

Thanks!

[DanTup]

@weinand

My minimalistic proposal:

VS Code just passes the currently open file as a new Source attribute to the evaluate request (and DA determines a thread behind the scenes automatically without prompting the user).

Is this something you're still considering? I'm passing a file:// URI as the context field for now, but it'd be much nicer if we could use something standard.

(we're also still interested in something like the getEvaluationContexts flow described above)