feat: add basic support for DAP (debug adapter protocol) #71
Conversation
|
@bishabosha @kubukoz maybe one of you would be willing to help with the PR? |
|
If all it needs is a self-contained classpath and the JVM port to attach to then maybe? if it can run standalone then cool. If it needs to discover classpath via BSP then I dont know, but perhaps java debug protocol handles all the runtime classpath |
|
@bishabosha - I described my approach in #53. I'm sorry, I'm not sure I understand your feedback. Are you writing about running the debugging process in the attach mode (remote debugging)? |
|
@susuro Sorry, I should have examined to check that this was a new PR with an actual implementation, and not a request for help to continuation of the older PR |
tgodzik
changed the title
| // and Scala DAP server has to be initialized by sending `debug-adapter-start` to Metals. | ||
| // The proxy should not interfere with the communication betweend the editor and language server. | ||
|
|
||
| import { Buffer } from "node:buffer"; |
There was a problem hiding this comment.
I think it's fine to turn it on by default, do we know how it was solved in Java extension? We can follow the same path
There was a problem hiding this comment.
As far as I'm aware, the proxy is turned on permanently for the Java extension.
tgodzik
left a comment
tgodzik
left a comment
There was a problem hiding this comment.
Great work! Thank you for working on this! Had a quick look and I think it looks good as far as I can say. Though I am neither an expert in Zed or Rust, so if anyone else could also take a look it would be awesome.
If no one is able, I will give it a go and merge if it works.
Thanks again!
|
Thank you for the review. I'll make the changes you suggested in #53. |
|
Both changes suggested in #53 have been implemented. |
|
For me the i tried launch with main class - works great with jumping to caught breakpoint in the editor, interactive console, variables and stack trace working fine - this is pretty awesome! (though console seems very slow when it drops down to non-fast mode), also completion suggestions dont work in the console, however i dont know if lsp is even possible to integrate here. However i also tried attach with a Cask (com-lihaoyi) app and it seems to attach as the breakpoint pauses the page loading until i click the "continue program" button however in this scenario the ui doesnt respond in zed to show any stack trace or variables, or which breakpoint is caught (only shows in the "Frames" view an error "invalid value: integer |
This error is caused by java-debug returning Anyway, most of the behavior you observed cannot be influenced by the extension. It's part of standard Zed's handling of DAP. |
bishabosha
left a comment
bishabosha
left a comment
There was a problem hiding this comment.
Thanks for all your efforts!
|
Unfortunately, I discovered a problem. |
|
I opened an issue in Zed: zed-idustries/zed#45209. |
|
How does the Java extension deal with that? |
Apparently doesn't. I reported the issue to Zed based on the Java extension to make the reproduction easier. We may still add a new setting to make DAP available, and leave the default mode as it is, without the proxy and without DAP support. |
makes sense, would you mind explaining in the docs how to turn it on? Or is there enough information for that already? |
|
@tgodzik - sorry for the confusion caused, but I have just committed a workaround for the issue. If any server properties (binary arguments) are specified by the user in settings.json, the extension defaults to starting Metals in direct mode, without the proxy and without DAP support. When the user tries to start the debug session, they receive a (hopefully) clear error message: DAP cannot start if any binary arguments for Metals are set in config file. This is further described in the README (Limitations and known problems). I think this is even better than the new setting for enabling the DAP - it will work by default if possible, but be excluded if the proxy cannot be started. @bishabosha - I would appreciate it if you could have another look at the recent change I made. |
|
Thank you for your support @bishabosha and @tgodzik |
3 participants
This is a preliminary version of DAP implementation, requested in #53.
As Zed extensions don't support sending requests to an LSP, a proxy server was required to allow such communication.
The solution is based on and partially uses code from https://github.com/zed-extensions/java.
This is a draft PR, not intended to be merged yet.