key map cleanup for popup

[danielo515]

Hello.
I am just enjoying a lot the new Layout component, specially for popups. The thing is that I am creating some complex layouts, and I am attaching popup specific mappings to easy navigation.
At first there was no problem with the only map that I added, which was esc, but as soon as I added more maps, they leaked and are being called in all windows where the buffer is displayed, despite the popup being closed (by layout unmount).

Do I need to manually remove the mappings? This particular behaviour only happens when I render an existing buffer inside the popup. My understanding is that, if the buffer exists I need to manually cleanup the bindigs? I wish this would happen automatically, but if not, let's document it.

Here is my code:

local Layout = require "nui.layout"
local Popup = require "nui.popup"

local win_options = {
  -- winblend = 10,
  winhighlight = "Normal:Normal,FloatBorder:FloatBorder",
}

local buf_options = {
  modifiable = true,
  readonly = false,
}

---Map the common set of keys for each popup
---@param pop any
---@param layout any
---@param alt_win number The window Id of the other popup
local function mapKeys(layout, pop, alt_win)
  local function goto_alt()
    vim.api.nvim_set_current_win(alt_win)
  end
  -- because when all you have is two windows, who cares about direction?
  pop:map("n", "<Tab>", goto_alt)
  pop:map("n", "<c-l>", goto_alt)
  pop:map("n", "<c-h>", goto_alt)
  pop:map("n", "<Esc>", function()
    layout:unmount()
  end)
end

---Mounts a split window with the two buffers loaded into each side.
---@param moduleName string
---@param buffers { implementation = number, interface = number } table with buffer ids
local function mount_window(moduleName, buffers)
  local p1 = Popup {
    enter = true,
    bufnr = buffers.implementation,
    win_options = win_options,
    buf_options = buf_options,
    border = {
      style = "double",
      text = {
        top = moduleName .. ".re",
        bottom = "<Esc> close this window",
      },
    },
  }

  local p2 = Popup {
    win_options = win_options,
    buf_options = buf_options,
    bufnr = buffers.interface,
    border = {
      style = "rounded",
      text = {
        top = moduleName .. ".rei",
      },
    },
  }

  local layout = Layout(
    { position = "50%", size = "90%", relative = "editor" },
    Layout.Box {
      Layout.Box({
        Layout.Box(p1, { size = "50%" }),
        Layout.Box(p2, { size = "50%" }),
      }, { size = "100%" }),
    }
  )

  layout:mount()
  -- Always map after mount
  mapKeys(layout, p1, p2.winid)
  mapKeys(layout, p2, p1.winid)
end

local function make_buffers(moduleName)
  local implementation = vim.fn.bufadd(moduleName .. ".re")
  local interface = vim.fn.bufadd(moduleName .. ".rei")

  vim.fn.bufload(implementation)
  vim.fn.bufload(interface)

  return { interface = interface, implementation = implementation }
end

---Creates a new module (interface + implementation) and opens
--it in a split floating window
local function create_module()
  local current_file = vim.fn.expand "%:t:r"
  D.vim.input(function(name)
    local path = vim.fn.expand "%:p:h"
    local buffers = make_buffers(path .. "/" .. name)
    mount_window(name, buffers)
  end, { current_file })
end

---Opens the current file as a module in a floating window
local function open_module()
  -- current file without extension
  local name = vim.fn.expand "%:p:r"
  local buffers = make_buffers(name)
  mount_window(name, buffers)
end

local M = {
  create_module = create_module,
  open_module = open_module,
}

return M

[MunifTanjim]

This is expected behavior. In Neovim, local keymaps are created against a specific buffer. So if you are supplying the buffer, you need to clean it up before unmounting. You can use popup:unmap for doing that.

[MunifTanjim]

The reason behind not doing anything to the externally managed buffer "automagically" is because it can lead to unwanted consequences.

Implementing unbind_on_exit = true would require nui.nvim to keep track of which keymaps were created using popup:map, because it can have other keymaps created outside nui.nvim too. And I try to not introduce specialized options like this, to keep things simple.

But nui.nvim is very extensible, and you can absolutely implement it for your specific project with something like this:

local ExtPopup = Popup:extend("ExtPopup")

function ExtPopup:init(options)
  ExtPopup.super.init(self, options)
  self._keymap_tracker = {}
end

function ExtPopup:map(mode, key, handler, opts)
  if not self._keymap_tracker[mode] then
    self._keymap_tracker[mode] = {}
  end
  self._keymap_tracker[mode][key] = true

  ExtPopup.super.map(self, mode, key, handler, opts)
end

function ExtPopup:unmap(mode, key)
  -- remove keymap from self._keymap_tracker

  ExtPopup.super.unmap(self, mode, key)
end

function ExtPopup:unmount()
  -- unmap keymaps from self._keymap_tracker

  ExtPopup.super.unmount(self)
end

[danielo515]

Ok, that is just perfect!
I saw this after asking you in the other issue, so we can consider this topic already answered in both places.
Thank you very much again. Will build something with it and I may add it to the wiki, if you think that is ok.

[danielo515]

Ok, this is what I ended with. So far, works like a charm:

local Popup = require "nui.popup"
local PopupKeys = Popup:extend "ExtPopup"

---A modifier version of the popup that tracks the keymaps
--you add and removes them on unmouns
---@param options table
function PopupKeys:init(options)
  PopupKeys.super.init(self, options)
  -- set some values for better editor integration
  ---@type table<string,table<string,boolean>>
  self._keymap_tracker = {
    n = {},
    i = {},
  }
end

function PopupKeys:map(mode, key, handler, opts)
  if not self._keymap_tracker[mode] then
    self._keymap_tracker[mode] = {}
  end
  self._keymap_tracker[mode][key] = true

  PopupKeys.super.map(self, mode, key, handler, opts)
end

function PopupKeys:unmap(mode, key)
  local mapping = self._keymap_tracker[mode][key]
  if mapping ~= nil then
    -- Remove from the tracker in case anyone removes it manually
    self._keymap_tracker[mode] = nil
  end
  PopupKeys.super.unmap(self, mode, key)
end

function PopupKeys:unmount()
  -- unmap keymaps from self._keymap_tracker
  for mode, maps in pairs(self._keymap_tracker) do
    for keymap, _ in pairs(maps) do
      PopupKeys.super.unmap(self, mode, keymap)
    end
  end
  PopupKeys.super.unmount(self)
end

return PopupKeys

[danielo515]

@MunifTanjim do you think it is worth add to the wiki?

[MunifTanjim]

Yep, sure. Somebody might stumble upon similar issue and find it useful.