Add configurable window width, and possibility to hide the terminal window

[tlj]

I never use the terminal window when debugging, so having it open and taking half the window size is a waste of space for me. This PR adds the option to change the size of the nvim-dap-view window:

require("dap-view").setup({windows = { width = 0.75 }})

Width is set to a percentage if the number is between 0 and 1, and otherwise it is set to number of columns. If the width is set to 1 it means 100%. This will use nvim_win_hide to hide the terminal window from view. The default is set to 0.5 which emulates the existing behaviour.

There is a comment in term_buf_win_init about not closing the term buffer, but I think hiding the window should be fine.

[catgoose]

I am thinking along the same lines concerning the terminal. Some adapters like delve do not support the integrated console. While you can run delve as an external process, attach to it with mode = "remote" and define an external terminal (mfussenegger/nvim-dap#1006 (comment)), in most cases I would never use the terminal.

As you can see here the fmt.Println statements print in the REPL and not in the terminal.

Perhaps we could disable it based on the adapter being used or let the user define keys based on git repo name/workspace?

[tlj]

Yes this is exactly my use case too, the delve adapter writes to repl so the terminal is just staying there unused.

If we were able to determine the usefulness of the terminal window based on capabilities or config sent from the adapter, that would be really neat, but I'm not sure if that is possible based on a quick look at the specs. Maybe having it as a config option, defaulting to showing it?

Something like:

setup({ hide_terminal_for_adapters = { "delve", "xdebug" }})

Having a list of adapters in the plugin itself would probably be hard to maintain.

[catgoose]

Yeah because if you look at the link in my comment, you end up having to use an external terminal like kitty to interact with delve.

Instead of the plug-in maintaining the list, the config opt could be bool or function. I think either way dap.session() you can get the configuration or adapter name. User might want to use neoconf or some other means to disable terminal, which is why I like the bool/function configuration.

[catgoose]

With #7, you can do something like this to close terminal.

vim.api.nvim_create_autocmd({ "FileType" }, {
  pattern = { "dap-view-term" },
  callback = function(event)
    vim.schedule(function()
      local session = require("dap").session()
      if session and session.filetype == "go" then
-- alternatively: if session and session.config and session.config.dlvToolPath then
        vim.api.nvim_buf_delete(event.buf, { force = true })
      end
    end)
  end,
})

[igorlfs]

Hey folks! Great discussion here!

I never use the terminal window when debugging, so having it open and taking half the window size is a waste of space for me.

Interestingly enough, I face the same situation using the js-debug-adapter. I thought it would be a matter before someone requested this feature and here we are :)

There is a comment in term_buf_win_init about not closing the term buffer, but I think hiding the window should be fine.

Given that delve seems to not use it at all, I'm not sure what would be the behavior if it was closed. Can you test it?

Perhaps we could disable it based on the adapter being used or let the user define keys based on git repo name/workspace?

That's the way to go. And fortunately, it should be somewhat straight forward. nvim-dap exposes the configuration when setting up the terminal_win_cmd. I still need to figure this out, though.

[igorlfs]

I wanna make a more throughout review later, but have you tested this with adapters other than delve?

[catgoose]

Given that delve seems to not use it at all, I'm not sure what would be the behavior if it was closed. Can you test it?

Today at work I have been using nvim-dap-view without the terminal throughout the day with the autocommand I posted and I haven't seen any issues.

If you look at the screenshot I posted, delve doesn't even output to the terminal, it shows in the dap repl.

[igorlfs]

Today at work I have been using nvim-dap-view without the terminal throughout the day with the autocommand I posted and I haven't seen any issues.

Yeah, now that I recall correctly, the reason why I wrote that comment about not closing the terminal was that it could have useful info, such as output. Since that's not the case with delve, it shouldn't cause any issues. Incidentally, though, I was trying to implement the "hiding the terminal for selected adapters" and ran into all sorts of issues, with nvim-dap popping out its default terminal all the time, so that had me worried something similar could happen in a scenario where delve was used. But I was being a dummy dummy, nothing to worry about.

[igorlfs]

Would an option to hide the terminal for selected adapters suffice your use cases? I'm not very leaned into having the width configurable (for now).

[catgoose]

Would an option to hide the terminal for selected adapters suffice your use cases? I'm not very leaned into having the width configurable (for now).

I feel like if you are setting a filetype for the terminal, then the user can use autocmd to close or resize. Then you don't have to add special configuration for different use cases, let the user take care of that.

vim.api.nvim_create_autocmd({ "FileType" }, {
  pattern = { "dap-view-term" },
  callback = function(evt)
    -- Scheduling is required because the event fires when filetype is set and it hasn't been assigned to a window yet.
    vim.schedule(function()
      local win_id = vim.fn.bufwinid(evt.buf)
      vim.api.nvim_win_set_width(win_id, math.floor(vim.o.columns * 0.25))
    end)
  end,
})

You could set different resize widths based on the adapter used from information gathered from dap.session()

[catgoose]

I was able to get the terminal window working with delve. This approach could be adapted for other adapters if they can accept TCP connections, but I have only tested with delve:

  if not dap.adapters.go then
    local function get_unused_port()
      local uv = vim.loop
      local server = uv.new_tcp()
      assert(server:bind("127.0.0.1", 0)) -- OS allocates an unused port
      local tcp_t = server:getsockname()
      server:close()
      assert(tcp_t and tcp_t.port > 0, "Failed to get an unused port")
      return tcp_t.port
    end
    dap.adapters.go = function(callback, config)
      get_unused_port()
      local port = config.port or get_unused_port()
      local state = require("dap-view.state")
      local bufnr = state.term_bufnr
      local winnr = state.term_winnr
      if not bufnr or not winnr then
        require("dap-view").open()
        bufnr = state.term_bufnr
        winnr = state.term_winnr
        if not bufnr or not winnr then
          -- dap-view not available, create window manually
          bufnr = vim.api.nvim_create_buf(true, false)
          winnr = vim.api.nvim_open_win(bufnr, false, {
            split = "below",
            win = -1,
            height = 12,
          })
        end
      end
      -- job start runs in current window.  Is there a way to start it in target
      -- window without switching?
      local current_winnr = vim.api.nvim_get_current_win()
      vim.api.nvim_set_current_win(winnr)
      vim.fn.jobstart({ "dlv", "dap", "-l", string.format("127.0.0.1:%d", port) }, {
        term = true,
        buffer = bufnr,
      })
      vim.api.nvim_set_current_win(current_winnr)
      vim.defer_fn(function()
        callback({ type = "server", host = "127.0.0.1", port = port })
      end, 100)
    end

    local dlvToolPath = vim.fn.exepath("dlv")
    dap.configurations.go = {
      {
        type = "go",
        name = "Debug main.go",
        request = "launch",
        showLog = true,
        program = "${workspaceFolder}/main.go",
        dlvToolPath = dlvToolPath,
        port = 11111,
      }
  }
end

Instead of getting winnr and bufnr from state, we could also check dap.defaults.fallback.terminal_win_cmd.

[tlj]

I think this is a very interesting discussion, but I'm afraid it might be moving somewhat out of scope of what I wanted to achieve with this PR. I think adding per-adapter configuration for what to show, callback functions, and so on should be a new PR. My main goal was to be able to resize the window, and secondary was the ability to resize it to take the full width, effectively hiding the terminal.

Another issue I realized with the current state is that the dap-view goes to the right of the terminal, so not only does half of my window width become useless, but it's also the most important part of the view (the left side). So I doubled down on this PR as a global window configuration and added a new option "position", which defaults to "right". This way if I want a global config where the dap-view goes to the left or above or under the terminal, then that is possible. In addition, doing width=1, position=bottom, gives the dap-view the full width under the current buffer.

I like to look at this as the global window config. Per-adapter config options could be added in another PR.

I feel like if you are setting a filetype for the terminal, then the user can use autocmd to close or resize. Then you don't have to add special configuration for different use cases, let the user take care of that.

I agree that this is really flexible and gives the user full flexibility to do whatever they like. Not sure if I think the user should be forced to do autocmds for such basic functionality on a globel level in the plugin though. It's a pattern which quickly leads to a lot of autocmds for various plugins.

Would an option to hide the terminal for selected adapters suffice your use cases? I'm not very leaned into having the width configurable (for now).

I think it could be a good option, but keep in mind that per-adapter config might need to do more than show/hide in the future.

Feel free to close this PR if you want to go in a different direction, I'm also happy to use the autocmd @catgoose posted above 😄