-
Notifications
You must be signed in to change notification settings - Fork 17
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
base: main
Are you sure you want to change the base?
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.
Implement #67