fix: use listener hooks for nvim-dap instead of monkey patching

Author: stevearc

doc/third_party.md

@@ -30,22 +30,24 @@ Or with options:
 ```lua
 require("lualine").setup({
   sections = {
-    lualine_x = { {
-      "overseer",
-      label = '',     -- Prefix for task counts
-      colored = true, -- Color the task icons and counts
-      symbols = {
-        [overseer.STATUS.FAILURE] = "F:",
-        [overseer.STATUS.CANCELED] = "C:",
-        [overseer.STATUS.SUCCESS] = "S:",
-        [overseer.STATUS.RUNNING] = "R:",
+    lualine_x = {
+      {
+        "overseer",
+        label = "", -- Prefix for task counts
+        colored = true, -- Color the task icons and counts
+        symbols = {
+          [overseer.STATUS.FAILURE] = "F:",
+          [overseer.STATUS.CANCELED] = "C:",
+          [overseer.STATUS.SUCCESS] = "S:",
+          [overseer.STATUS.RUNNING] = "R:",
+        },
+        unique = false, -- Unique-ify non-running task count by name
+        name = nil, -- List of task names to search for
+        name_not = false, -- When true, invert the name search
+        status = nil, -- List of task statuses to display
+        status_not = false, -- When true, invert the status search
       },
-      unique = false,     -- Unique-ify non-running task count by name
-      name = nil,         -- List of task names to search for
-      name_not = false,   -- When true, invert the name search
-      status = nil,       -- List of task statuses to display
-      status_not = false, -- When true, invert the status search
-    } },
+    },
   },
 })
 ```
@@ -109,7 +111,7 @@ local Overseer = {
 To run all neotest processes using overseer, add it as a custom consumer:
 
 ```lua
-require('neotest').setup({
+require("neotest").setup({
   consumers = {
     overseer = require("neotest.consumers.overseer"),
   },
@@ -119,7 +121,7 @@ require('neotest').setup({
 This will automatically hook `neotest.run` and force it to use overseer to run tests wherever possible. If you would instead like to only use the overseer consumer explicitly, you can disable the monkey patching:
 
 ```lua
-require('neotest').setup({
+require("neotest").setup({
   consumers = {
     overseer = require("neotest.consumers.overseer"),
   },
@@ -137,15 +139,15 @@ neotest.overseer.run({})
 You can customize the default components of neotest tasks by setting the `default_neotest` component alias (when unset it maps to `default`).
 
 ```lua
-require('overseer').setup({
+require("overseer").setup({
   component_aliases = {
     default_neotest = {
       "on_output_summarize",
       "on_exit_set_status",
       "on_complete_notify",
       "on_complete_dispose",
     },
-  }
+  },
 })
 ```
 
@@ -170,43 +172,36 @@ require("neotest").setup({
 
 ## DAP
 
-If you have both overseer and [nvim-dap](https://github.com/mfussenegger/nvim-dap) installed, overseer will automatically run the `preLaunchTask` and `postDebugTask` when present in a debug configuration.
-
-I also recommend that you configure nvim-dap to use overseer's json decoder, since it supports JSON5 (comments and trailing commas)
-
-```lua
-require("dap.ext.vscode").json_decode = require("overseer.json").decode
-```
+If you have both overseer and [nvim-dap](https://github.com/mfussenegger/nvim-dap) installed, overseer will automatically run the `preLaunchTask` and `postDebugTask` when present in a debug configuration. No special configuration or action is needed.
 
-A note about lazy loading: make sure you load nvim-dap before overseer because overseer will attempt
-to patch nvim-dap inside `overseer.setup()`. Alternatively, you can disable the automatic support
+For lazy-loading, you may wish to avoid loading `nvim-dap` when overseer is loaded. If so, you can disable DAP support initially:
 
 ```lua
 require("overseer").setup({
   dap = false,
 })
 ```
 
-And enable the integration manually later, such as when nvim-dap is loaded
+And enable the integration manually later, such as when `nvim-dap` is loaded:
 
 ```lua
-require("overseer").patch_dap(true)
+require("overseer").enable_dap()
 ```
 
 ## ToggleTerm
 
 If you use [toggleterm](https://github.com/akinsho/toggleterm.nvim), you can use the built-in "toggleterm" strategy to allow your tasks to be in a terminal buffer owned by toggleterm. You can use your existing toggleterm keybinds to pull up long-running tasks started with overseer. You can set it up with defaults using:
 
 ```lua
-require('overseer').setup({
+require("overseer").setup({
   strategy = "toggleterm",
 })
 ```
 
 You can also configure the behavior a bit more:
 
 ```lua
-require('overseer').setup({
+require("overseer").setup({
   strategy = {
     "toggleterm",
     -- load your default shell before starting the task
@@ -233,7 +228,7 @@ require('overseer').setup({
     -- command to run when the terminal is created. Combine with `use_shell`
     -- to run a terminal command before starting the task
     on_create = nil,
-  }
+  },
 })
 ```
 
@@ -246,12 +241,12 @@ More documentation on this strategy can be found [here](strategies.md#toggleterm
 Overseer has built-in support for [resession.nvim](https://github.com/stevearc/resession.nvim).
 
 ```lua
-require('resession').setup({
+require("resession").setup({
   extensions = {
     overseer = {
       -- customize here
-    }
-  }
+    },
+  },
 })
 ```
 
@@ -288,7 +283,7 @@ require("auto-session").setup({
       for _, task in ipairs(overseer.list_tasks({})) do
         task:dispose(true)
       end
-    end
+    end,
   },
   post_restore_cmds = {
     function()

lua/overseer/dap.lua

@@ -20,74 +20,68 @@ local function get_task(name, config, cb)
   require("overseer").run_template(args, cb)
 end
 
-M.wrap_run = function(daprun)
-  return function(config, opts)
-    dap.listeners.after.event_terminated["overseer"] = function()
-      if config.postDebugTask then
-        log:trace("Running DAP postDebugTask %s", config.postDebugTask)
-        get_task(config.postDebugTask, config, function(task, err)
-          if err then
-            log:error("Could not run postDebugTask %s", config.postDebugTask)
-          elseif task then
-            task:start()
-          end
-        end)
-      end
-    end
+M.listener = function(config)
+  dap.listeners.after.event_terminated.overseer = nil
+  if not config.preLaunchTask and not config.postDebugTask then
+    return config
+  end
 
-    if config.preLaunchTask then
-      log:trace("Running DAP preLaunchTask %s", config.preLaunchTask)
-      get_task(config.preLaunchTask, config, function(task, err)
-        if not task then
-          log:error("Could not run preLaunchTask %s: %s", config.preLaunchTask, err)
-          return
+  if config.postDebugTask then
+    dap.listeners.after.event_terminated.overseer = function()
+      log:debug("Running DAP postDebugTask %s", config.postDebugTask)
+      get_task(config.postDebugTask, config, function(task, err)
+        if err then
+          log:error("Could not run postDebugTask %s", config.postDebugTask)
+        elseif task then
+          task:start()
         end
+      end)
+    end
+  end
 
-        -- Non-background task with problemMatcher will trigger both on_result and on_complete so use the first one
-        local done = false
-        local cleanup
+  if config.preLaunchTask then
+    log:debug("Running DAP preLaunchTask %s", config.preLaunchTask)
+    local co = coroutine.running()
+    get_task(config.preLaunchTask, config, function(task, err)
+      if not task then
+        log:error("Could not run preLaunchTask %s: %s", config.preLaunchTask, err)
+        return
+      end
 
-        local function on_done(ok)
-          if done then
-            return
-          end
-          done = true
+      -- Non-background task with problemMatcher will trigger both on_result and on_complete so use the first one
+      local on_done
+      on_done = function(ok)
+        on_done = function() end
 
-          if ok then
-            daprun(config, opts)
-          else
-            vim.notify(
-              string.format(
-                "Failed to launch debugger; preLaunchTask '%s' failed",
-                config.preLaunchTask
-              ),
-              vim.log.levels.ERROR
-            )
-          end
-          vim.schedule(cleanup)
+        if not ok then
+          vim.notify(
+            string.format("DAP preLaunchTask '%s' failed", config.preLaunchTask),
+            vim.log.levels.ERROR
+          )
         end
+        coroutine.resume(co)
+      end
 
-        local function on_complete(_, status)
-          on_done(status == STATUS.SUCCESS)
-        end
+      local function on_complete(_, status)
+        on_done(status == STATUS.SUCCESS)
+        return false
+      end
 
-        local function on_result()
-          -- We get the on_result callback from background tasks once they hit their end pattern
-          on_done(task.status ~= STATUS.FAILURE)
-        end
+      local function on_result()
+        -- We get the on_result callback from background tasks once they hit their end pattern
+        on_done(task.status ~= STATUS.FAILURE)
+        return false
+      end
 
-        task:subscribe("on_complete", on_complete)
-        task:subscribe("on_result", on_result)
-        cleanup = function()
-          task:unsubscribe("on_complete", on_complete)
-          task:unsubscribe("on_result", on_result)
-        end
-        task:start()
-      end)
-    else
-      daprun(config, opts)
-    end
+      task:subscribe("on_complete", on_complete)
+      task:subscribe("on_result", on_result)
+      task:start()
+    end)
+
+    coroutine.yield()
   end
+
+  return config
 end
 
 return M

lua/overseer/init.lua

@@ -254,35 +254,38 @@ end
 
 ---Add support for preLaunchTask/postDebugTask to nvim-dap
 ---@private
+---@deprecated
 ---@param enabled boolean
 M.patch_dap = function(enabled)
+  M.enable_dap(enabled)
+end
+
+---Add support for preLaunchTask/postDebugTask to nvim-dap
+---This is enabled by default when you call overseer.setup() unless you set `dap = false`
+---@param enabled boolean
+M.enable_dap = function(enabled)
+  if enabled == nil then
+    enabled = true
+  end
   if not enabled and not package.loaded.dap then
     return
   end
   local ok, dap = pcall(require, "dap")
   if not ok then
     return
   end
-  if type(dap.run) == "table" then
-    if not enabled then
-      dap.run = dap.run.original
+  if enabled then
+    dap.listeners.on_config.overseer = require("overseer.dap").listener
+
+    -- If the user has not overridden the DAP json decoder, use ours since it supports JSON5
+    local vscode = require("dap.ext.vscode")
+    if vscode.json_decode == vim.json.decode then
+      vscode.json_decode = require("overseer.json").decode
     end
-    return
-  elseif not enabled then
-    return
+  else
+    dap.listeners.on_config.overseer = nil
+    dap.listeners.after.event_terminated.overseer = nil
   end
-  local daprun = dap.run
-  dap.run = setmetatable({
-    wrapper = nil,
-    original = daprun,
-  }, {
-    __call = function(self, config, opts)
-      if not self.wrapper then
-        self.wrapper = require("overseer.dap").wrap_run(daprun)
-      end
-      self.wrapper(config, opts)
-    end,
-  })
 end
 
 ---Initialize overseer
@@ -297,7 +300,7 @@ M.setup = function(opts)
   end
   opts = opts or {}
   create_commands()
-  M.patch_dap(opts.dap ~= false)
+  M.enable_dap(opts.dap)
   pending_opts = opts
   if initialized then
     do_setup()