Proxy dap requests to upstream clients #77
Conversation
|
@RemcoSmitsDev This is still a WIP, but I would appreciate any feedback you have |
|
I think you are going in the right direction, one thing that came to my mind is that we now also have to sync the capabilities. Because we are using this to determine if we are sending a specific request or not, or use another request that is supported. |
|
|
||
| let task = self.request_dap::<SetVariable>(client_id, arguments, cx); | ||
|
|
||
| cx.background_executor().spawn(async move { |
There was a problem hiding this comment.
I think this wrapping it inside a background task is not needed, as you already do this inside the request_dap method.
There was a problem hiding this comment.
I tried doing this
task.map(|res| res.map(|response| response.variables)) before, but the return type wasn't the same as the function. I do think you're right that we shouldn't do create two tasks, I'm going to change the function's return type instead. Thanks for the feedback!!
| cx: &mut ModelContext<Self>, | ||
| ) -> Task<Result<R::Response>> | ||
| where | ||
| <R as dap::requests::Request>::Response: 'static, |
There was a problem hiding this comment.
Do we have to specify the lifetime here? Not sure if this is really needed.
There was a problem hiding this comment.
Without it the compiler returns an error when we try to build.
I think it's because we're moving that type into it's own thread and it needs to have ownership over it's fields to guarantee that they're still exist in the Task.
Thank you for catching this!! I didn't even think about that |
RemcoSmitsDev
force-pushed
the
debugger
branch
from
b2a58c6 to
7ced61d
Compare
WIP