-
Couldn't load subscription status.
- Fork 28
Add support for debugger #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| We require contributors to sign our Contributor License Agreement, and we don't have @DeityLamb on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
| @cla-bot check |
| The cla-bot has been summoned, and re-checked this pull request! |
| I’ve implemented downloading the debug plugin, Right now, the DAP port is passed via a I haven’t been able to get the debugger fully running yet, still figuring out exactly what’s required for that. |
| Okay, I’ve made some progress with the debugger I added a schema for the request options, and I managed to try running the debugger without it crashing For now, it seems that the debugger, for some reason, can’t detect the classpaths in the project [ { "adapter": "Java", "request": "launch", "label": "run", "mainClass": "org.example.App", "classPaths": ["$Auto"], "stopOnEntry": true } ]And for every request I get DAP logs, it seems it cannot connect to the process |
| Oh, looks like "classPaths": ["app/build/classes/java/main"]It seems this part will also need to be implemented Well, the debugger itself works, and that’s good |
| Okay, I’ll summarize what’s ready so far. Downloading and integrating the java-debug plugin. We obtain the port for launching the debugger via a direct request, so there’s no issue with that anymore. I tried adding auto-detection of the main class, project, and class paths ( The current filling logic is as follows: The result is matched against our current config (if it has any values), then we take the first Scope determination in class paths:
// https://github.com/microsoft/vscode-java-debug/blob/main/src/configurationProvider.ts#L518 let scope = { if classpaths.iter().any(|class| class == TEST_SCOPE) { Some("test".to_string()) } else if classpaths.iter().any(|class| class == AUTO_SCOPE) { None } else if classpaths.iter().any(|class| class == RUNTIME_SCOPE) { Some("runtime".to_string()) } else { None } };Then we call We remove duplicates and aliases from the class paths. When testing, this approach didn’t work for every project (particularly when running tests). @gayanper could you suggest how this might be better implemented? |
| If i understand the last part which is related to running tests The test execution support is implemented differently in a different extension in vscode. When running tests we have to do the following
Given that majority of real world projects are either maven or gradle, we could basically use either or those CLIs to run the tests in debug mode and use that port to connect the zed DAP client. |
| Okay, it looks like I’ve finished the main functionality Unfortunately, we can’t implement the I think it might be possible to create another workaround for task/test runners, Currently, scenarios for running the application as well as attaching to a process by PID or port are working Open for review @valentinegb @gayanper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good to Me from a design point of view, specially how Java LS and DAP is setup. I cannot comment must on the Rust and Zed specific things though since I'm new as well.
| @valentinegb It would be great if we could merge these changes soon |
| When testing this on my machine, the language server fails to start with the following logged by Zed: |
| Fixed, the minimum required Node version is now v16.x |
| Any plans to release this soon? |
| @valentinegb It would also be great if you could let me know when you’ll have time for a review I’m really eager to start implementing the contentProvider for sources and decompiled dependencies, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Really sorry I'm late to review this again ^^'
I think as far as I'm concerned, this PR looks good and I approve it :>
On that note though I should probably say that this project should look for a new maintainer. Honestly, I'm not even really a Java developer and I was just interested in working on this project to improve the experience for myself while I had to use Java for a class. But, now I don't have that class, so I can't help but let my interest dwindle, considering I haven't even touched Java in a while. I'm not a member of @zed-extensions so I will leave it to them to find another maintainer
| @DeityLamb i am getting this error when i was trying the extension out today
|
| @pranav-dream11 This happens when the LSP isn’t running. Try restarting the JDTLS extension and wait a bit before debugging. Properly handling all lifecycles with all the required workarounds is nearly impossible, sorry :) I’ll try to figure something out with it later |
| @DeityLamb thanks, it works amazingly well |
| I am using this debug configuration and this is my class When trying to run using debugger, getting following error Env is windows and extensions installed are Java v6.4.1, Java with Eclipse JDTLS v0.2.5. |
Implement #67