mfussenegger / nvim-dap

Debug Adapter Protocol client implementation for Neovim
GNU General Public License v3.0
5.46k stars 194 forks source link

Make `require 'dap.ui.widgets'.hover()` jump to float only when calling twice #1194

Open lawrence-laz opened 5 months ago

lawrence-laz commented 5 months ago

Problem Statement

Currently require 'dap.ui.widgets'.hover() jumps to the floating window on the first call, which requires to keep doing <C-w>q each time. Having to do this multiple times in short duration becomes very cumbersome (dK is mapped to dap hover):

dK<C-w>q ww dK<C-W>q ww dK<C-W>q ww  (and so on)

Possible Solutions

Make it act like vim.lsp.buf.hover():

--- Displays hover information about the symbol under the cursor in a floating
--- window. Calling the function twice will jump into the floating window.
function M.hover()
    -- ...
end

This would simplify the previous use case to:

dK ww dK ww dK

And for when you need to actually jump the floating window it's not hard to press the shortcut twice, as your fingers are already in place, as opposed to keep switching back an forth between dK and <C-w>q.

Considered Alternatives

I tried looking for a way to configure this just for me, as I don't really mind the defaults being as they are now, but couldn't find how to do it.

mfussenegger commented 5 months ago

Would using dap.ui.widgets.preview() work instead? Or something like this:

local dap = require("dap")
vim.keymap.set("n", "<leader>di", function()
  dap.repl.open()
  dap.repl.execute(vim.fn.expand("<cexpr>"))
end)
vim.keymap.set("v", "<leader>di", function()
  -- getregion requires nvim 0.10
  local lines = vim.fn.getregion(vim.fn.getpos("."), vim.fn.getpos("v"))
  dap.repl.open()
  dap.repl.execute(table.concat(lines, "\n"))
end)

The focus-by-default for hover is going to stay, because it prioritizes the use-case where you need to expand some of the variables to drill down to the info you need. preview covers the "peek" use-case.

lawrence-laz commented 5 months ago

These are ok, but not quite like built-in lsp hovering.

I think I managed to get what I wanted by modifying a copy of dap.ui.widgets.hover() to actually use a window created by vim.lsp.util.open_floating_preview:

function Dap_better_hover(expr, winopts)
    local value = dap_eval_expression(expr)

    local bufnr, winid = vim.lsp.util.open_floating_preview({}, "dap-float", {
        focusable = true,
        close_events = { 'CursorMoved', 'BufHidden', 'InsertCharPre' },
        focus_id = 'dappp',
        focus = true,
        width = 100,
        height = 5,
    })

    local buffer_lines = vim.api.nvim_buf_get_lines(bufnr, 1, 999, false)
    if (#buffer_lines ~= 0) then
        -- If buffer already existed, then we just jumped into it and can return early to avoid creating duplicated content.
        return
    end

    -- Buffer options
    vim.bo[bufnr].bufhidden = "wipe"
    vim.bo[bufnr].filetype = "dap-float"
    vim.bo[bufnr].modifiable = false
    vim.bo[bufnr].buftype = "nofile"
    vim.api.nvim_buf_set_name(bufnr, 'dap-hover-' .. tostring(bufnr) .. ': ' .. value)

    -- Window options
    vim.wo[winid].scrolloff = 0

    -- Key mappings for the buffer
    vim.api.nvim_buf_set_keymap(bufnr, "n", "<CR>", "<Cmd>lua require('dap.ui').trigger_actions({ mode = 'first' })<CR>",
        {})
    vim.api.nvim_buf_set_keymap(bufnr, "n", "a", "<Cmd>lua require('dap.ui').trigger_actions()<CR>", {})
    vim.api.nvim_buf_set_keymap(bufnr, "n", "o", "<Cmd>lua require('dap.ui').trigger_actions()<CR>", {})
    vim.api.nvim_buf_set_keymap(bufnr, "n", "<2-LeftMouse>", "<Cmd>lua require('dap.ui').trigger_actions()<CR>", {})

    local view = require("dap.ui.widgets").builder(require("dap.ui.widgets").expression)
        .new_buf(function() return bufnr end)
        .new_win(require("dap.ui.widgets").with_resize(function() return winid end))
        .build()
    view.open(value)
    return view
end

The only problem is that it uses a local function eval_expression, which I don't think I can access from my config, so I had to copy that as well.

Other than that it now acts as I wanted it, so I think we can close the issue.

mfussenegger commented 5 months ago

I'll keep it open if you don't mind. The use-case sounds common enough to warrant an easier way to set it up and refining the widget api/system has been on my todo for 1.0

igorlfs commented 4 months ago

On that topic, I do think a nice default mapping for widgets would be to map q to <C-w>q, since it's a common pattern that many plugins use for floating windows. Any thoughts?