nvim-dap
nvim-dap copied to clipboard
Add hooks for preLaunchTask and postDebugTask
Problem Statement
The launch.json file format allows specifying a preLaunchTask and a postDebugTask, which will run an asynchronous job before and after the debugging session. At the moment, these values are ignored by nvim-dap, which is unfortunate because they may be a required step for making the debug session work.
Possible Solutions
I have written a task-manager plugin that has support for the VS Code tasks.json file format, and currently have an integration for nvim-dap that wires these properties up correctly. However, it unfortunately relies on some pretty gross monkey patching. I'm proposing adding a hook for dap.run that will, when present, call a user-defined function and pass it a callback that will resume execution when called. That would be enough for me to get rid of the monkey patching, and it's generic enough that it doesn't tie nvim-dap to overseer, or to the launch.json format.
There has been some prior discussion about this (#191), where the suggestion was to put the task running logic into the adapter. I believe that it makes more sense to add this to dap core instead of the adapters because the preLaunchTask/tasks.json format is universal across adapters.
Considered Alternatives
We could continue with the monkey patching. It's more brittle, but it does currently work in most cases.
I'm happy to do the work here, btw, I just want to get approval for the approach.
I agree that some extension mechanism to handle this use-case would make sense. Do you have a proposal for the API?
Another use-case I can imagine is the codelldb adapter where the vscode extension handles a cargo property on the client side.
Currently handling this would also work via the adapter on_config mechanism, but it doesn't compose well - e.g. a cpp focused dap extension and a rust focused extension may both want to define the codelldb adapter, which would then conflict.
So I'm thinking this could either be something where extensions can register handles for specific properties, or it would be a more general pluggable config pre-processing mechanism that's global instead of per adapter. Not sure how to deal with conflicts if multiple extensions want to handle the same property, or if it is a generic pre-processing how the ordering would work out.
So I'm thinking this could either be something where extensions can register handles for specific properties, or it would be a more general pluggable config pre-processing mechanism that's global instead of per adapter.
I would personally lean more towards a general pluggable config pre-processor. As you mentioned, adapters already have the ability to do this via the enrich_config/on_config function. Conceptually, this feels fairly similar to "listeners", except that it actually blocks execution instead of just being a fire-and-forget event. Perhaps the API could look similar?
local function my_hook() end
dap.on_pre_run["overseer"] = my_hook
Or since this is actually very similar to the adapter enrich_config, maybe we could re-use that name:
dap.enrich_config["overseer"] = my_hook
I don't know if we need to worry too much about conflicting extensions or the ordering of these methods. I've seen a lot of pub/sub APIs in various systems (including vim's autocmds) with minimal or no control over ordering and that usually isn't an issue. But if you think it's something that should be possible, the API could be changed to support internals that are more list-like.
local function my_hook() end
dap.add_pre_run_hook(my_hook) -- implicit ordering; first added, first called
dap.remove_pre_run_hook(my_hook)
This would allow users to roughly enforce some sort of order in a pinch, and the API could easily be extended in the future to take an options table with a priority if it needs to become a real feature.
I'm trying to get debugging to work with Azure Functions with Node/TypeScript and this sounds exactly like what I need. This is the default configuration for debugging Azure Functions that VSC**e generates and it would be awesome if nvim-dap supported it:
.vscode/launch.json:
{
"version": "0.2.0",
"configurations": [
{
"name": "Attach to Node Functions",
"type": "node",
"request": "attach",
"port": 9229,
"preLaunchTask": "func: host start"
}
]
}
.vscode/tasks.json:
{
"version": "2.0.0",
"tasks": [
{
"type": "func",
"label": "func: host start",
"command": "host start",
"problemMatcher": "$func-node-watch",
"isBackground": true,
"dependsOn": "npm build (functions)"
},
{
"type": "shell",
"label": "npm build (functions)",
"command": "npm run build",
"dependsOn": "npm install (functions)",
"problemMatcher": "$tsc"
},
{
"type": "shell",
"label": "npm install (functions)",
"command": "npm install"
},
{
"type": "shell",
"label": "npm prune (functions)",
"command": "npm prune --production",
"dependsOn": "npm build (functions)",
"problemMatcher": []
}
]
}
@melkster this should already work, as overseer does currently support integrating with nvim-dap. This issue is about cleaning up and formalizing the interface, because at the moment it relies on monkey patching.
If you encounter issues, please file them on the overseer repo. I suspect you might because while I did add support for Microsoft Azure functions, I don't use Azure myself and so couldn't test it.
@stevearc Awesome, you're right! I've created an issue here: #762. Do you think it's an issue with nvim-dap or with overseer.nvim?
Please don't derail this issue with another topic. This is about an extension point for config pre-processing. Nothing else.
@mfussenegger do you have any comments about the API proposal above?
This would be useful for things such as Ruby on Rail's server debugging, where after the debugging session ends, the server should be stopped.
This helps avoid the need to manually stop the server, as it's hogging the port required to start another debug session: https://github.com/suketa/nvim-dap-ruby/issues/25
I would love to have an option for asynchronous tasks as well. But I guess what I'm looking for is more of a on_run hook. I don't think on_config or enrich_config can handle multiple configurations in java. At least I couldn't get it working. At the moment, I update the dap.configurations.java and dap.adapter.java on LspAttachevent. However, if a new main class is created or changed, then those are not reflected. Best option is to wrap functions like overseer has done and call dap.run in the callback.
@stevearc could you check if https://github.com/mfussenegger/nvim-dap/pull/1241 would cover your requirements?
Made a follow up that moves on_config from dap.providers to dap.listeners. See https://github.com/mfussenegger/nvim-dap/pull/1248
Closing this for now but please let me know if you encounter any limitations with this system.
The new API is great! I refactored my integration to use it instead of monkey patching https://github.com/stevearc/overseer.nvim/commit/ecdfbac807652a374414d3d6f3e5b3af201f884d Thanks for adding support!
The new API is great! I refactored my integration to use it instead of monkey patching stevearc/overseer.nvim@ecdfbac Thanks for adding support!
Nice to hear.
In case you haven't already: Maybe worth keeping the monkey-patch as fallback and guarding the new mechanism behind a if dap.listeners.on_config then check to keep it working for people who stick to released/tagged nvim-dap versions. At least until I've tagged a new release that includes the functionality.