Merge pull request #18 from mhiro2/refactor/toggle-api

refactor(toggle): Rename `toggle_visibility` to `toggle` and preserve popup IDs

Author: mhiro2

README.md

@@ -73,7 +73,7 @@ Using lazy.nvim:
     vim.keymap.set("n", "<leader>pm", function() peekstack.peek.marks_buffer() end)
 
     -- Utility: temporarily hide/show all popups in current stack
-    vim.keymap.set("n", "<leader>ph", "<cmd>PeekstackToggleVisibility<cr>", { desc = "Peekstack: toggle visibility" })
+    vim.keymap.set("n", "<leader>ph", "<cmd>PeekstackToggle<cr>", { desc = "Peekstack: toggle" })
   end,
 }
 ```
@@ -112,7 +112,7 @@ Built-in provider names:
 - `:PeekstackRestorePopup` — restore the last closed popup (undo close)
 - `:PeekstackRestoreAllPopups` — restore all closed popups
 - `:PeekstackCloseAll` — close all popups in the current stack
-- `:PeekstackToggleVisibility` — temporarily hide/show all popups in the current stack
+- `:PeekstackToggle` — temporarily hide/show all popups in the current stack
 - `:PeekstackHistory` — show popup history and select to restore
 - `:PeekstackQuickPeek [provider]` — quick peek without stacking (default: `lsp.definition`, accepts any registered provider)
 

doc/peekstack.txt

@@ -291,7 +291,7 @@ Commands are registered after `setup()` is called.
 :PeekstackCloseAll                            *:PeekstackCloseAll*
     Close all popups in the current stack.
 
-:PeekstackToggleVisibility                *:PeekstackToggleVisibility*
+:PeekstackToggle                                    *:PeekstackToggle*
     Temporarily hide or show all popups in the current stack. When hidden,
     popup windows are closed but the stack state is preserved. Toggling
     again recreates the windows in their original layout. Pushing a new

lua/peekstack/commands.lua

@@ -13,7 +13,7 @@ local COMMAND_NAMES = {
   "PeekstackHistory",
   "PeekstackCloseAll",
   "PeekstackQuickPeek",
-  "PeekstackToggleVisibility",
+  "PeekstackToggle",
 }
 
 ---@param session PeekstackSession|table
@@ -174,8 +174,8 @@ function M.setup()
     require("peekstack.core.stack").close_all()
   end, {})
 
-  vim.api.nvim_create_user_command("PeekstackToggleVisibility", function()
-    local toggled = require("peekstack.core.stack").toggle_visibility()
+  vim.api.nvim_create_user_command("PeekstackToggle", function()
+    local toggled = require("peekstack.core.stack").toggle()
     if not toggled then
       notify.info("No popups in the current stack")
     end

lua/peekstack/core/popup.lua

@@ -199,8 +199,10 @@ function M.create(location, opts)
   end
   set_cursor(winid, location, line_offset)
 
-  local id = next_id
-  next_id = next_id + 1
+  local id = opts.id or next_id
+  if not opts.id then
+    next_id = next_id + 1
+  end
 
   local popup = {
     id = id,

lua/peekstack/core/stack.lua

@@ -281,7 +281,7 @@ function M.push(location, opts)
   local stack = ensure_stack()
   -- Auto-show hidden stack before pushing a new popup
   if stack.hidden then
-    M.toggle_visibility(stack.root_winid)
+    M.toggle(stack.root_winid)
   end
   local model = popup.create(location, create_opts)
   if not model then
@@ -543,6 +543,7 @@ end
 ---@return PeekstackPopupModel?
 local function reopen_popup(item, stack)
   local reopen_opts = {
+    id = item.id,
     buffer_mode = item.buffer_mode or "copy",
     origin_winid = stack.root_winid,
     parent_popup_id = item.parent_popup_id,
@@ -554,10 +555,7 @@ local function reopen_popup(item, stack)
   if not model then
     return nil
   end
-  model.id = item.id
   model.pinned = item.pinned or false
-  vim.b[model.bufnr].peekstack_popup_id = model.id
-  vim.w[model.winid].peekstack_popup_id = model.id
   return model
 end
 
@@ -849,7 +847,7 @@ end
 ---When shown, popup windows are recreated from the stored models.
 ---@param winid? integer
 ---@return boolean
-function M.toggle_visibility(winid)
+function M.toggle(winid)
   deps()
   local stack = ensure_stack(winid)
   if #stack.popups == 0 then

tests/toggle_visibility_spec.lua tests/toggle_spec.luatests/toggle_visibility_spec.lua renamed to tests/toggle_spec.lua

@@ -3,7 +3,7 @@ local config = require("peekstack.config")
 local events = require("peekstack.core.events")
 local helpers = require("tests.helpers")
 
-describe("stack.toggle_visibility", function()
+describe("stack.toggle", function()
   before_each(function()
     stack._reset()
     config.setup({})
@@ -14,7 +14,7 @@ describe("stack.toggle_visibility", function()
     local s = stack.current_stack()
     -- Restore visibility so windows can be closed normally
     if s.hidden then
-      stack.toggle_visibility()
+      stack.toggle()
     end
     for i = #s.popups, 1, -1 do
       stack.close(s.popups[i].id)
@@ -23,7 +23,7 @@ describe("stack.toggle_visibility", function()
   end)
 
   it("returns false on empty stack", function()
-    assert.is_false(stack.toggle_visibility())
+    assert.is_false(stack.toggle())
     assert.is_false(stack.is_hidden())
   end)
 
@@ -34,7 +34,7 @@ describe("stack.toggle_visibility", function()
     assert.is_not_nil(m1)
     assert.is_not_nil(m2)
 
-    assert.is_true(stack.toggle_visibility())
+    assert.is_true(stack.toggle())
     assert.is_true(stack.is_hidden())
 
     -- Windows should be gone but popups remain in the stack
@@ -53,11 +53,11 @@ describe("stack.toggle_visibility", function()
     assert.is_not_nil(m2)
 
     -- Hide
-    stack.toggle_visibility()
+    stack.toggle()
     assert.is_true(stack.is_hidden())
 
     -- Show
-    assert.is_true(stack.toggle_visibility())
+    assert.is_true(stack.toggle())
     assert.is_false(stack.is_hidden())
 
     local s = stack.current_stack()
@@ -76,8 +76,8 @@ describe("stack.toggle_visibility", function()
     local id1 = m1.id
     local id2 = m2.id
 
-    stack.toggle_visibility()
-    stack.toggle_visibility()
+    stack.toggle()
+    stack.toggle()
 
     local s = stack.current_stack()
     assert.equals(id1, s.popups[1].id)
@@ -88,7 +88,7 @@ describe("stack.toggle_visibility", function()
     local loc = helpers.make_location()
     stack.push(loc)
 
-    stack.toggle_visibility()
+    stack.toggle()
     assert.is_true(stack.is_hidden())
 
     -- Pushing should auto-show
@@ -109,7 +109,7 @@ describe("stack.toggle_visibility", function()
     stack.push(loc)
     stack.push(loc)
 
-    stack.toggle_visibility()
+    stack.toggle()
     assert.is_true(stack.is_hidden())
 
     stack.close_all()
@@ -124,7 +124,7 @@ describe("stack.toggle_visibility", function()
     stack.push(loc)
     stack.push(loc)
 
-    stack.toggle_visibility()
+    stack.toggle()
     assert.is_true(stack.is_hidden())
 
     -- reflow_all should skip hidden popups (winid=nil) safely
@@ -140,13 +140,54 @@ describe("stack.toggle_visibility", function()
     end
   end)
 
+  it("close by id works after hide/show cycle", function()
+    local loc = helpers.make_location()
+    local m1 = stack.push(loc)
+    local m2 = stack.push(loc)
+
+    -- Hide then show
+    stack.toggle()
+    stack.toggle()
+
+    -- close should find the popup by its original id
+    assert.is_true(stack.close(m2.id))
+    assert.is_true(stack.close(m1.id))
+
+    local s = stack.current_stack()
+    assert.equals(0, #s.popups)
+  end)
+
+  it("keymaps reference correct popup id after hide/show cycle", function()
+    local loc = helpers.make_location()
+    local m1 = stack.push(loc)
+
+    local original_id = m1.id
+
+    -- Hide then show (recreates buffer + keymaps)
+    stack.toggle()
+    stack.toggle()
+
+    local s = stack.current_stack()
+    local restored = s.popups[1]
+    assert.equals(original_id, restored.id)
+
+    -- The close keymap should work via the buffer variable
+    assert.equals(original_id, vim.b[restored.bufnr].peekstack_popup_id)
+
+    -- Simulate what the close keymap does: resolve + close by popup_id
+    local found = stack.find_by_id(original_id)
+    assert.is_not_nil(found)
+    assert.equals(restored.bufnr, found.bufnr)
+    assert.is_true(stack.close(original_id))
+  end)
+
   it("does not leak popups to history when hiding", function()
     local loc = helpers.make_location()
     stack.push(loc)
     stack.push(loc)
 
     local history_before = #stack.history_list()
-    stack.toggle_visibility()
+    stack.toggle()
     local history_after = #stack.history_list()
 
     assert.equals(history_before, history_after)