diff --git a/README.md b/README.md index edba2d6..ee65fb3 100644 --- a/README.md +++ b/README.md @@ -365,12 +365,16 @@ A session holds one stack: the popups of the current window (or of the window th belongs to), oldest first, keeping the newest `persist.max_items`. Only locations, titles, pins, buffer modes and parent links are saved; edits made in copy-mode popups are not. Restoring a session adds its popups to the current window's stack and keeps the popups already there. +Each popup comes back in the buffer mode it was opened with, whatever `ui.popup.buffer_mode` is set to. > [!WARNING] > Persistence uses repository storage when the current working directory is inside a git repository. > Outside a git repository, sessions fall back to cwd-based storage. > The storage is resolved when a save, delete, or rename is requested, so changing directory while -> it is still being written does not move it to another repository. +> it is still being written does not move it to another repository. Session commands resolve it, and +> the window whose stack they act on, when the command runs: changing directory or window while a +> name prompt, session picker, or delete confirmation is open does not redirect them, and a save or +> restore is dropped if that window has been closed by then. Each git repository (each worktree, found by looking upward from the current window's working directory, so `:lcd` / `:tcd` count) has its own storage file, and session names are scoped to it. @@ -383,6 +387,11 @@ directory, so `:lcd` / `:tcd` count) has its own storage file, and session names - **Unreadable storage**: if the storage file cannot be read (an I/O error, invalid JSON, or a version this release does not know), peekstack warns and refuses to save, delete, or rename sessions in it instead of replacing it with an empty store. Fix or move the file to recover. +- **Several Neovim instances**: instances sharing a storage file take turns through a `.lock` + file held for each read and write, so saving different sessions at the same time keeps both. Saving + the same name keeps the later save. If another instance holds the lock for more than a second, the + operation fails with a warning instead of writing. A lock is removed only once the instance that + created it has exited. > [!IMPORTANT] > Sessions are written as plain JSON under `vim.fn.stdpath("state") .. "/peekstack/"`. Each entry @@ -398,11 +407,13 @@ When `persist.auto.enabled = true`, peekstack can automatically restore and save - **Restore** on `VimEnter` / `DirChanged` (only when the stack is empty if `restore_if_empty = true`) - **Save** on `PeekstackPush` / `PeekstackClose` / `PeekstackRestorePopup` with a debounce -- **Save on leave** on `VimLeavePre` if `save_on_leave = true` +- **Save on leave** on `VimLeavePre` if `save_on_leave = true` (works on its own: with `save = false` + the stack is written only when leaving) The auto session holds a single stack: the one that changed most recently, saved to the repository that was current when it changed. Closing every popup of that stack saves it empty, so nothing is -restored next time. +restored next time. Quick peek popups never join a stack, so opening or closing one does not touch +the auto session. Leaving Neovim saves that stack synchronously, even if the cursor is in another window. It skips the save if no stack changed during the session, and keeps the stored session as-is once that stack's diff --git a/doc/peekstack.txt b/doc/peekstack.txt index 69e94fc..389a323 100644 --- a/doc/peekstack.txt +++ b/doc/peekstack.txt @@ -485,7 +485,8 @@ window the stack view belongs to), oldest first, keeping the newest `persist.max_items`. Only locations, titles, pins, buffer modes and parent links are saved; edits made in copy-mode popups are not. Restoring a session adds its popups to the current window's stack and keeps the popups -already there. +already there. Each popup comes back in the buffer mode it was opened +with, whatever `ui.popup.buffer_mode` is set to. Persistence uses repository storage when the current working directory is inside a git repository. Outside a git repository, it falls back to @@ -494,7 +495,11 @@ upward from the current window's working directory, so |:lcd| and |:tcd| count) has its own storage file, and session names are scoped to it. The storage is resolved when a save, delete, or rename is requested, so changing directory while it is still being written does not move it to -another repository. +another repository. Session commands resolve it, and the window whose +stack they act on, when the command runs: changing directory or window +while a name prompt, session picker, or delete confirmation is open does +not redirect them, and a save or restore is dropped if that window has +been closed by then. Empty stacks: Saving an empty stack replaces the session with an empty one; restoring @@ -513,6 +518,14 @@ Unreadable storage: save, delete, or rename sessions in it instead of replacing it with an empty store. Fix or move the file to recover. +Several Neovim instances: + Instances sharing a storage file take turns through a `.lock` + file held for each read and write, so saving different sessions at + the same time keeps both. Saving the same name keeps the later save. + If another instance holds the lock for more than a second, the + operation fails with a warning instead of writing. A lock is removed + only once the instance that created it has exited. + `persist.session.default_name` is used when a name is omitted, unless `persist.session.prompt_if_missing` is true. @@ -523,12 +536,14 @@ Auto persistence (`persist.auto`) requires `persist.enabled = true`: restore_if_empty only restore when the stack is empty save save on PeekstackPush/Close/RestorePopup (debounced) debounce_ms debounce time for auto save - save_on_leave save on VimLeavePre + save_on_leave save on VimLeavePre (also with `save = false`, in + which case the stack is written only when leaving) The auto session holds a single stack: the one that changed most recently, saved to the repository that was current when it changed. Closing every popup of that stack saves it empty, so nothing is restored -next time. +next time. Quick peek popups never join a stack, so opening or closing one +does not touch the auto session. Leaving Neovim saves that stack synchronously, even if the cursor is in another window. It skips the save if no stack changed during the session, diff --git a/lua/peekstack/commands.lua b/lua/peekstack/commands.lua index 59c5ffd..2a9b544 100644 --- a/lua/peekstack/commands.lua +++ b/lua/peekstack/commands.lua @@ -67,12 +67,32 @@ local function complete_session_names(arg_lead) return filter_by_prefix(list_session_names(), arg_lead) end +---Storage and stack a session command acts on. +---Prompts and pickers answer asynchronously, and the user may change +---directory or window before they do, so both are fixed when the command +---runs and handed to every later step. +---@class PeekstackSessionCommandTarget +---@field store_path string +---@field root_winid integer + +---@return PeekstackSessionCommandTarget +local function capture_session_target() + return { + store_path = require("peekstack.persist.orchestrator").store_path(), + -- The resolved stack root, so closing the popup or stack view the command + -- ran from does not lose the stack; saves and restores are dropped once + -- the root window itself is gone. + root_winid = require("peekstack.persist.sessions").resolve_root_winid() --[[@as integer]], + } +end + ---Confirm and delete a named session. ---@param name string -local function confirm_delete_session(name) +---@param target PeekstackSessionCommandTarget +local function confirm_delete_session(name, target) vim.ui.select({ "Yes", "No" }, { prompt = "Delete session '" .. name .. "'?" }, function(choice) if choice == "Yes" then - require("peekstack.persist").delete_session(name) + require("peekstack.persist").delete_session(name, { store_path = target.store_path }) end end) end @@ -92,11 +112,15 @@ function M.setup() local cfg = require("peekstack.config").get() if not name and cfg.persist.session and cfg.persist.session.prompt_if_missing then + local target = capture_session_target() vim.ui.input( { prompt = "Session name: ", default = cfg.persist.session.default_name or "default" }, function(input) if input and input ~= "" then - require("peekstack.persist").save_current(input) + require("peekstack.persist").save_current(input, { + store_path = target.store_path, + root_winid = target.root_winid, + }) end end ) @@ -116,7 +140,9 @@ function M.setup() vim.api.nvim_create_user_command("PeekstackListSessions", function() local persist = require("peekstack.persist") + local target = capture_session_target() persist.list_sessions({ + store_path = target.store_path, on_done = function(sessions) local names = vim.tbl_keys(sessions) table.sort(names) @@ -137,7 +163,7 @@ function M.setup() ) vim.ui.select({ "Restore", "Info only" }, { prompt = info }, function(action) if action == "Restore" then - persist.restore(selected) + persist.restore(selected, { store_path = target.store_path, root_winid = target.root_winid }) end end) end) @@ -147,12 +173,14 @@ function M.setup() vim.api.nvim_create_user_command("PeekstackDeleteSession", function(opts) local name = opts.args and opts.args ~= "" and opts.args or nil + local target = capture_session_target() if name then - confirm_delete_session(name) + confirm_delete_session(name, target) return end require("peekstack.persist").list_sessions({ + store_path = target.store_path, on_done = function(sessions) local names = vim.tbl_keys(sessions) table.sort(names) @@ -164,7 +192,7 @@ function M.setup() if not selected then return end - confirm_delete_session(selected) + confirm_delete_session(selected, target) end) end, }) diff --git a/lua/peekstack/persist/auto.lua b/lua/peekstack/persist/auto.lua index 91762a6..25c5177 100644 --- a/lua/peekstack/persist/auto.lua +++ b/lua/peekstack/persist/auto.lua @@ -146,20 +146,24 @@ function M.maybe_restore() return true end +---Record a stack change and debounce a save of it when `auto.save` is on. +---The change is tracked either way, so the leave-time save still knows +---which stack to write when only `auto.save_on_leave` is enabled. ---@param opts? { root_winid?: integer } ----@return boolean +---@return boolean scheduled whether a debounced save was scheduled function M.schedule_save(opts) if not is_enabled() then return false end - local cfg = config.get() - if not cfg.persist.auto.save then + local target = capture_target(opts and opts.root_winid or nil) + if not target then return false end + tracked_target = target - local target = capture_target(opts and opts.root_winid or nil) - if not target then + local cfg = config.get() + if not cfg.persist.auto.save then return false end @@ -172,7 +176,6 @@ function M.schedule_save(opts) flush_pending() end pending_target = target - tracked_target = target local debounce_ms = tonumber(cfg.persist.auto.debounce_ms) or 1000 if save_timer then @@ -255,13 +258,21 @@ function M.setup() }) end - if cfg.persist.auto.save then + -- Changes are observed for the leave-time save too, which needs to know + -- the stack to write even when debounced saves are off. + if cfg.persist.auto.save or cfg.persist.auto.save_on_leave then vim.api.nvim_create_autocmd("User", { group = group, pattern = { "PeekstackPush", "PeekstackClose", "PeekstackRestorePopup" }, callback = function(args) - local root_winid = args.data and args.data.root_winid or nil - M.schedule_save({ root_winid = root_winid }) + local data = args.data or {} + -- Quick peek popups never join a stack, so opening or closing one + -- leaves nothing to save; treating it as a change would overwrite the + -- session with the (usually empty) stack of the window it opened in. + if data.ephemeral then + return + end + M.schedule_save({ root_winid = data.root_winid }) end, }) end diff --git a/lua/peekstack/persist/lock.lua b/lua/peekstack/persist/lock.lua new file mode 100644 index 0000000..c277e89 --- /dev/null +++ b/lua/peekstack/persist/lock.lua @@ -0,0 +1,220 @@ +local fs = require("peekstack.util.fs") +local notify = require("peekstack.util.notify") + +---Advisory lock that serializes store updates across Neovim processes. +---The in-process update queue cannot see writes from another Neovim using +---the same store, so each read-modify-write holds `.lock`, created +---with O_EXCL, from before the read until after the write. +local M = {} + +---How long to wait for another process to release the lock. +local ACQUIRE_TIMEOUT_MS = 1000 +local RETRY_INTERVAL_MS = 5 +---A lock whose owner pid cannot be read (it may not be written yet) is only +---treated as left behind once it is older than this. +local STALE_AFTER_S = 10 + +---Locks this process holds, keyed by lock file path. +---@type table +local held = {} + +---Release every lock this process holds. +function M.release_all() + for _, release in pairs(held) do + release() + end +end + +-- An async write cut short by quitting must not leave its lock behind. +vim.api.nvim_create_autocmd("VimLeave", { + group = vim.api.nvim_create_augroup("PeekstackPersistLock", { clear = true }), + callback = M.release_all, +}) + +---@param path string store file path +---@return string +function M.lock_path(path) + return path .. ".lock" +end + +---Identity of a lock file, to tell the file this process created from one +---another process created after taking the lock over as stale. +---@param stat uv.fs_stat.result? +---@return string? +local function identity(stat) + if not stat then + return nil + end + return string.format("%d:%d:%d:%d", stat.dev, stat.ino, stat.mtime.sec, stat.mtime.nsec) +end + +---@param lock string +---@param id string +---@return fun() release +local function make_release(lock, id) + local released = false + local function release() + if released then + return + end + released = true + held[lock] = nil + if identity(vim.uv.fs_stat(lock)) == id then + vim.uv.fs_unlink(lock) + end + end + held[lock] = release + return release +end + +---@param lock string +---@return integer? +local function read_pid(lock) + local fd = vim.uv.fs_open(lock, "r", 438) + if not fd then + return nil + end + local content = vim.uv.fs_read(fd, 32, 0) + vim.uv.fs_close(fd) + return tonumber(content) +end + +---Whether the lock was left behind by a process that exited while holding +---it. A lock whose owner is still running is never taken over, however +---long it is held. +---@param lock string +---@param stat uv.fs_stat.result +---@return boolean +local function is_abandoned(lock, stat) + local pid = read_pid(lock) + if pid then + local alive, _, code = vim.uv.kill(pid, 0) + -- EPERM: the process exists but belongs to another user. + return not alive and code ~= "EPERM" + end + return os.time() - stat.mtime.sec > STALE_AFTER_S +end + +---Remove an abandoned lock. +---Takeovers are serialized through `.takeover`, also created with +---O_EXCL: only the holder of that guard removes a lock it does not own, so +---the lock it checks cannot be replaced before it removes it. Without the +---guard, an instance that found the same abandoned lock could delete the +---lock another one has acquired since. +---@param lock string +---@param stat uv.fs_stat.result the abandoned lock as observed +local function take_over(lock, stat) + local guard = lock .. ".takeover" + local fd = vim.uv.fs_open(guard, "wx", 420) + if not fd then + -- The guard is held for a stat and an unlink only; one this old was left + -- by an instance that exited in between. Two instances removing the same + -- abandoned guard at once could still both end up holding one; that + -- needs a crash inside that short window and is not guarded against. + local guard_stat = vim.uv.fs_stat(guard) + if guard_stat and os.time() - guard_stat.mtime.sec > STALE_AFTER_S then + vim.uv.fs_unlink(guard) + end + return + end + vim.uv.fs_close(fd) + + local current = vim.uv.fs_stat(lock) + if current and identity(current) == identity(stat) and is_abandoned(lock, current) then + vim.uv.fs_unlink(lock) + end + vim.uv.fs_unlink(guard) +end + +---Try to take the lock once. +---@param lock string +---@return (fun())? release set when the lock was taken +---@return boolean retry whether another attempt may succeed +local function try_acquire(lock) + local fd, err, code = vim.uv.fs_open(lock, "wx", 420) + if fd then + -- The pid tells other instances whether the owner is still running. + local written = vim.uv.fs_write(fd, tostring(vim.uv.os_getpid()), 0) + local id = identity(vim.uv.fs_fstat(fd)) + vim.uv.fs_close(fd) + if not written or not id then + vim.uv.fs_unlink(lock) + notify.warn("Failed to lock session data: " .. lock) + return nil, false + end + return make_release(lock, id), false + end + + if code == "EEXIST" then + local stat = vim.uv.fs_stat(lock) + if stat and is_abandoned(lock, stat) then + take_over(lock, stat) + end + return nil, true + end + if code == "ENOENT" then + local ok = pcall(fs.ensure_dir, vim.fs.dirname(lock)) + if ok then + return nil, true + end + end + notify.warn(string.format("Failed to lock session data: %s (%s)", lock, tostring(err))) + return nil, false +end + +---@param path string +local function warn_timeout(path) + notify.warn("Timed out waiting for another Neovim to finish writing session data: " .. path) +end + +---Take the lock for the store at `path`, retrying while another process +---holds it. `on_done` receives the release function, or nil when the lock +---could not be taken within ACQUIRE_TIMEOUT_MS. +---@param path string store file path +---@param on_done fun(release: (fun())?) +function M.acquire(path, on_done) + local lock = M.lock_path(path) + local deadline = vim.uv.hrtime() + ACQUIRE_TIMEOUT_MS * 1e6 + local function attempt() + local release, retry = try_acquire(lock) + if release then + on_done(release) + return + end + if retry and vim.uv.hrtime() < deadline then + vim.defer_fn(attempt, RETRY_INTERVAL_MS) + return + end + if retry then + warn_timeout(path) + end + on_done(nil) + end + attempt() +end + +---Blocking variant of `acquire`. It sleeps between attempts without +---processing events, so no callback can start another update while this +---one waits for the lock. +---@param path string store file path +---@return (fun())? release nil when the lock could not be taken +function M.acquire_sync(path) + local lock = M.lock_path(path) + local deadline = vim.uv.hrtime() + ACQUIRE_TIMEOUT_MS * 1e6 + while true do + local release, retry = try_acquire(lock) + if release then + return release + end + if not retry then + return nil + end + if vim.uv.hrtime() >= deadline then + warn_timeout(path) + return nil + end + vim.uv.sleep(RETRY_INTERVAL_MS) + end +end + +return M diff --git a/lua/peekstack/persist/orchestrator.lua b/lua/peekstack/persist/orchestrator.lua index c91da92..382bc3f 100644 --- a/lua/peekstack/persist/orchestrator.lua +++ b/lua/peekstack/persist/orchestrator.lua @@ -1,6 +1,7 @@ local config = require("peekstack.config") local fs = require("peekstack.util.fs") local cache = require("peekstack.persist.cache") +local lock = require("peekstack.persist.lock") local migrate = require("peekstack.persist.migrate") local store = require("peekstack.persist.store") local notify = require("peekstack.util.notify") @@ -142,7 +143,14 @@ local function run_next_update() end update_running = true + ---@type (fun())? + local release = nil + local function finish(success) + if release then + release() + release = nil + end -- Release the queue before the callback so an update it enqueues (e.g. a -- sync save from a PeekstackSave handler) can run instead of waiting on -- this finished one. Earlier queued updates still go first. @@ -183,10 +191,25 @@ local function run_next_update() end end + -- Hold the store lock from the read through the write, so another + -- Neovim cannot write in between and have that write overwritten with + -- data read before it. if update.sync then + release = lock.acquire_sync(update.path) + if not release then + finish(false) + return + end apply(M.read_sync(update.path)) else - M.read_async(update.path, apply) + lock.acquire(update.path, function(acquired) + if not acquired then + finish(false) + return + end + release = acquired + M.read_async(update.path, apply) + end) end end @@ -201,7 +224,8 @@ end ---Asynchronously read, mutate and write back store data at `path`. ---Updates are serialized: each one reads the file only after the previous ---write finished, so overlapping save/delete/rename calls cannot clobber each ----other with a stale snapshot (read-modify-write lost update). +---other with a stale snapshot (read-modify-write lost update). Across Neovim +---processes the same holds through the store lock. ---`mutate` returns true to persist the change or false to abort without ---writing; `on_done` receives whether the data was written successfully. ---@param path string @@ -257,11 +281,13 @@ function M.update_sync(path, mutate) return false end ----Reset the in-memory session cache and drop queued updates. +---Reset the in-memory session cache, drop queued updates and release the +---store lock an in-flight update holds. function M.reset_cache() cache.reset() update_queue = {} update_running = false + lock.release_all() end ---@param path string diff --git a/lua/peekstack/persist/service.lua b/lua/peekstack/persist/service.lua index c5c17cc..7212e44 100644 --- a/lua/peekstack/persist/service.lua +++ b/lua/peekstack/persist/service.lua @@ -108,6 +108,13 @@ local function notify_save_result(success, name, items, silent) end end +---@param silent boolean +local function notify_missing_window(silent) + if not silent then + notify.warn("The window of the session's stack was closed") + end +end + ---Save the current stack to persistent storage with optional name. ---@param name? string ---`store_path` pins the store file; it defaults to the current repository's @@ -129,7 +136,13 @@ function M.save_current(name, opts) end local resolved_name = sessions.resolve_name(name) - local items = sessions.collect_items(opts and opts.root_winid or nil) + local root_winid = sessions.resolve_root_winid(opts and opts.root_winid or nil) + if not root_winid then + notify_missing_window(silent) + finish(false) + return + end + local items = sessions.collect_items(root_winid) local path = opts and opts.store_path or orchestrator.store_path() if sync then @@ -152,8 +165,10 @@ function M.save_current(name, opts) end ---Restore a named session from persistent storage. +---`store_path` pins the store file; it defaults to the current repository's +---store, resolved when the restore is requested. ---@param name? string ----@param opts? { root_winid?: integer, silent?: boolean, on_done?: fun(restored: boolean) } +---@param opts? { root_winid?: integer, store_path?: string, silent?: boolean, on_done?: fun(restored: boolean) } function M.restore(name, opts) local silent = opts and opts.silent or false local on_done = opts and opts.on_done or nil @@ -172,7 +187,13 @@ function M.restore(name, opts) -- Freeze the target stack now: the read below is async and the user may -- move to another window before the session is restored. local root_winid = sessions.resolve_root_winid(opts and opts.root_winid or nil) - orchestrator.refresh_cache_async(orchestrator.store_path(), function(data) + if not root_winid then + notify_missing_window(silent) + finish(false) + return + end + local path = opts and opts.store_path or orchestrator.store_path() + orchestrator.refresh_cache_async(path, function(data) if not data then finish(false) return @@ -236,7 +257,7 @@ function M.restore(name, opts) end ---List all saved sessions. ----@param opts? { on_done?: fun(sessions: table), silent?: boolean } +---@param opts? { on_done?: fun(sessions: table), store_path?: string, silent?: boolean } ---@return table function M.list_sessions(opts) local on_done = opts and opts.on_done or nil @@ -251,7 +272,7 @@ function M.list_sessions(opts) return {} end - local path = orchestrator.store_path() + local path = opts and opts.store_path or orchestrator.store_path() if on_done then orchestrator.refresh_cache_async(path, function(data) on_done(data and data.sessions or {}) @@ -265,13 +286,14 @@ end ---Delete a named session. ---@param name string -function M.delete_session(name) +---@param opts? { store_path?: string } +function M.delete_session(name, opts) if not orchestrator.ensure_enabled() then return end local found = false - orchestrator.update_async(orchestrator.store_path(), function(data) + orchestrator.update_async(opts and opts.store_path or orchestrator.store_path(), function(data) found = sessions.delete(data, name) if not found then notify.warn("Session not found: " .. name) @@ -295,7 +317,8 @@ end ---Rename a session. ---@param from string ---@param to string -function M.rename_session(from, to) +---@param opts? { store_path?: string } +function M.rename_session(from, to, opts) if not orchestrator.ensure_enabled() then return end @@ -306,7 +329,7 @@ function M.rename_session(from, to) end local renamed = false - orchestrator.update_async(orchestrator.store_path(), function(data) + orchestrator.update_async(opts and opts.store_path or orchestrator.store_path(), function(data) local result = sessions.rename(data, from, to) renamed = result.ok if result.err == "missing" then diff --git a/lua/peekstack/persist/sessions.lua b/lua/peekstack/persist/sessions.lua index 62def7c..72f3c25 100644 --- a/lua/peekstack/persist/sessions.lua +++ b/lua/peekstack/persist/sessions.lua @@ -19,23 +19,28 @@ function M.resolve_name(name) return "default" end ----Resolve the root window id to use when collecting stack items. +---Resolve the stack root a session operation acts on. +---Without `root_winid` this is the current window's stack. An explicit +---`root_winid` that no longer exists yields nil instead of falling back to the +---current window, whose stack is unrelated. ---@param root_winid? integer ----@return integer +---@return integer? function M.resolve_root_winid(root_winid) - if root_winid and type(root_winid) == "number" and vim.api.nvim_win_is_valid(root_winid) then + if root_winid == nil then + return stack.get_root_winid(vim.api.nvim_get_current_win()) + end + if type(root_winid) == "number" and vim.api.nvim_win_is_valid(root_winid) then return stack.get_root_winid(root_winid) end - - return stack.get_root_winid(vim.api.nvim_get_current_win()) + return nil end ----Collect the current stack items (truncated to `persist.max_items`). ----@param root_winid? integer +---Collect the items of a stack (truncated to `persist.max_items`). +---@param root_winid integer resolved stack root ---@return PeekstackSessionItem[] function M.collect_items(root_winid) local data_items = {} - for _, popup in ipairs(stack.list(M.resolve_root_winid(root_winid))) do + for _, popup in ipairs(stack.list(root_winid)) do data_items[#data_items + 1] = { uri = popup.location.uri, range = popup.location.range, @@ -44,7 +49,9 @@ function M.collect_items(root_winid) ts = os.time(), popup_id = popup.id, pinned = popup.pinned or nil, - buffer_mode = popup.buffer_mode ~= "copy" and popup.buffer_mode or nil, + -- Saved even when it matches the default, so a restore under another + -- `ui.popup.buffer_mode` brings the popup back the way it was opened. + buffer_mode = popup.buffer_mode, parent_popup_id = popup.parent_popup_id, } end diff --git a/tests/commands_spec.lua b/tests/commands_spec.lua index 3902875..21e8a59 100644 --- a/tests/commands_spec.lua +++ b/tests/commands_spec.lua @@ -314,4 +314,205 @@ describe("peekstack.commands", function() names = vim.fn.getcompletion("PeekstackQuickPeek ", "cmdline") assert.is_true(vim.list_contains(names, "custom.test")) end) + + describe("session commands across a directory change", function() + local fs = require("peekstack.util.fs") + local orchestrator = require("peekstack.persist.orchestrator") + local stack = require("peekstack.core.stack") + local store = require("peekstack.persist.store") + local original_cwd = nil + local repo_a = nil + local repo_b = nil + local store_a = nil + local store_b = nil + ---@type { prompt: string, on_choice: fun(choice: any) }[] + local selects = {} + + ---@param dir string + local function cd(dir) + vim.api.nvim_cmd({ cmd = "cd", args = { dir } }, {}) + end + + ---@param dir string + ---@return string + local function make_repo(dir) + vim.fn.mkdir(dir .. "/.git", "p") + vim.fn.writefile({ "line1", "line2", "line3" }, dir .. "/file.lua") + return vim.fn.resolve(dir) + end + + ---@param dir string + ---@param count integer + ---@return PeekstackSession + local function session_with(dir, count) + local items = {} + for i = 1, count do + items[i] = { + uri = vim.uri_from_fname(dir .. "/file.lua"), + range = { start = { line = i - 1, character = 0 }, ["end"] = { line = i - 1, character = 1 } }, + provider = "test", + ts = os.time(), + } + end + return { items = items, meta = { created_at = os.time(), updated_at = os.time() } } + end + + ---@param path string + ---@return table + local function stored_sessions(path) + local data = store.read_sync(path) + return data and data.sessions or {} + end + + ---@param prompt_pattern string + ---@return fun(choice: any) + local function take_select(prompt_pattern) + local ok = vim.wait(3000, function() + return #selects > 0 + end, 10) + assert.is_true(ok, "Timed out waiting for vim.ui.select") + local entry = table.remove(selects, 1) + assert.is_truthy(entry.prompt:find(prompt_pattern)) + return entry.on_choice + end + + before_each(function() + config.setup({ persist = { enabled = true } }) + commands.setup() + persist._reset_cache() + stack._reset() + selects = {} + vim.notify = function() end + vim.ui.select = function(_items, opts, on_choice) + selects[#selects + 1] = { prompt = opts.prompt, on_choice = on_choice } + end + + original_cwd = vim.fn.getcwd() + local base = vim.fn.tempname() + repo_a = make_repo(base .. "/repo_a") + repo_b = make_repo(base .. "/repo_b") + + cd(repo_b) + store_b = orchestrator.store_path() + assert.is_true(store.write_sync(store_b, { version = 2, sessions = { shared = session_with(repo_b, 2) } })) + cd(repo_a) + store_a = orchestrator.store_path() + assert.are_not.equal(store_a, store_b) + assert.is_true(store.write_sync(store_a, { version = 2, sessions = { shared = session_with(repo_a, 1) } })) + end) + + after_each(function() + stack._reset() + cd(original_cwd) + fs._reset_repo_root_cache() + vim.fn.delete(store_a) + vim.fn.delete(store_b) + vim.fn.delete(vim.fs.dirname(repo_a), "rf") + persist._reset_cache() + end) + + it("deletes the named session of the repository the confirmation was asked in", function() + vim.cmd("PeekstackDeleteSession shared") + local confirm = take_select("Delete session 'shared'") + cd(repo_b) + confirm("Yes") + + assert.is_true(vim.wait(3000, function() + return stored_sessions(store_a).shared == nil + end, 10)) + assert.is_not_nil(stored_sessions(store_b).shared) + end) + + it("deletes a picked session of the repository the command ran in", function() + vim.cmd("PeekstackDeleteSession") + local pick = take_select("Delete session") + cd(repo_b) + pick("shared") + take_select("Delete session 'shared'")("Yes") + + assert.is_true(vim.wait(3000, function() + return stored_sessions(store_a).shared == nil + end, 10)) + assert.is_not_nil(stored_sessions(store_b).shared) + end) + + it("restores a listed session from its repository into the window that listed it", function() + local winid = vim.api.nvim_get_current_win() + vim.cmd("PeekstackListSessions") + local pick = take_select("Select a session") + cd(repo_b) + vim.cmd("vsplit") + local other = vim.api.nvim_get_current_win() + pick("shared") + take_select("shared: 1 items")("Restore") + + assert.is_true(vim.wait(3000, function() + return #stack.list(winid) == 1 + end, 10)) + assert.equals(0, #stack.list(other)) + vim.api.nvim_win_close(other, true) + end) + + it("saves a prompted session to the repository and stack the command ran in", function() + config.setup({ persist = { enabled = true, session = { prompt_if_missing = true } } }) + local original_input = vim.ui.input + local on_input = nil + vim.ui.input = function(_opts, cb) + on_input = cb + end + local winid = vim.api.nvim_get_current_win() + assert.is_not_nil(stack.push({ + uri = vim.uri_from_fname(repo_a .. "/file.lua"), + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 1 } }, + provider = "test", + }, { root_winid = winid })) + + local ok, err = pcall(function() + vim.cmd("PeekstackSaveSession") + assert.is_not_nil(on_input) + cd(repo_b) + vim.cmd("vsplit") + on_input("prompted") + + assert.is_true(vim.wait(3000, function() + return stored_sessions(store_a).prompted ~= nil + end, 10)) + assert.equals(1, #stored_sessions(store_a).prompted.items) + assert.is_nil(stored_sessions(store_b).prompted) + end) + vim.ui.input = original_input + vim.cmd("silent! only") + if not ok then + error(err) + end + end) + + it("drops a prompted save once the window it ran in was closed", function() + config.setup({ persist = { enabled = true, session = { prompt_if_missing = true } } }) + local original_input = vim.ui.input + local on_input = nil + vim.ui.input = function(_opts, cb) + on_input = cb + end + + local ok, err = pcall(function() + vim.cmd("vsplit") + local closing = vim.api.nvim_get_current_win() + vim.cmd("PeekstackSaveSession") + assert.is_not_nil(on_input) + vim.api.nvim_win_close(closing, true) + on_input("orphaned") + + vim.wait(60, function() + return false + end, 10) + assert.is_nil(stored_sessions(store_a).orphaned) + end) + vim.ui.input = original_input + vim.cmd("silent! only") + if not ok then + error(err) + end + end) + end) end) diff --git a/tests/persist_auto_spec.lua b/tests/persist_auto_spec.lua index 449aa62..b0ee4a1 100644 --- a/tests/persist_auto_spec.lua +++ b/tests/persist_auto_spec.lua @@ -255,4 +255,123 @@ describe("peekstack.persist.auto", function() assert.equals(0, #calls) assert.is_false(auto.schedule_save({ root_winid = closing })) end) + + describe("with real popups and store", function() + local store = require("peekstack.persist.store") + local temp_file = nil + + ---@param winid? integer + ---@param opts? table + ---@return PeekstackPopupModel + local function push_popup(winid, opts) + local model = stack.push({ + uri = vim.uri_from_fname(temp_file), + range = { start = { line = 1, character = 0 }, ["end"] = { line = 1, character = 1 } }, + provider = "test", + }, vim.tbl_extend("force", { root_winid = winid }, opts or {})) + assert.is_not_nil(model) + return model + end + + ---@return PeekstackSessionItem[]? + local function saved_items() + local data = store.read_sync(store_path_for(repo)) + local session = data and data.sessions.auto + return session and session.items or nil + end + + ---@param count integer + local function wait_for_saved_items(count) + local ok = vim.wait(3000, function() + local items = saved_items() + return items ~= nil and #items == count + end, 10) + assert.is_true(ok, "Timed out waiting for " .. count .. " saved item(s)") + end + + ---Long enough for a debounced save to have started, were one scheduled. + local function settle() + vim.wait(200, function() + return false + end, 10) + end + + local repo_seq = 0 + + before_each(function() + persist._reset_cache() + -- A store per test: a save still in flight when a test ends must not + -- land in the next test's store. + repo_seq = repo_seq + 1 + repo = string.format("/tmp/peekstack-test-repo-real-%d-%d", vim.uv.os_getpid(), repo_seq) + vim.fn.delete(store_path_for(repo)) + temp_file = vim.fn.tempname() .. ".lua" + vim.fn.writefile({ "line1", "line2", "line3" }, temp_file) + end) + + after_each(function() + vim.api.nvim_create_augroup("PeekstackPersistAuto", { clear = true }) + pcall(stack.close_ephemerals) + stack._reset() + vim.fn.delete(store_path_for(repo)) + if temp_file then + vim.fn.delete(temp_file) + end + persist._reset_cache() + end) + + it("keeps the saved session when a quick peek opens and closes elsewhere", function() + auto.setup() + local first = vim.api.nvim_get_current_win() + push_popup(first) + wait_for_saved_items(1) + + vim.api.nvim_set_current_win(first) + vim.cmd("vsplit") + local second = vim.api.nvim_get_current_win() + local quick = push_popup(second, { stack = false }) + assert.is_true(quick.ephemeral) + settle() + stack.close(quick.id) + settle() + + assert.equals(1, #stack.list(first)) + assert.equals(0, #stack.list(second)) + assert.equals(1, #saved_items()) + end) + + it("saves on leave when only save_on_leave is enabled", function() + config.setup({ + persist = { + enabled = true, + auto = { enabled = true, restore = false, save = false, save_on_leave = true, debounce_ms = 20 }, + }, + }) + auto.setup() + push_popup(vim.api.nvim_get_current_win()) + settle() + assert.is_nil(saved_items()) + + vim.cmd("vsplit") + vim.api.nvim_exec_autocmds("VimLeavePre", { group = "PeekstackPersistAuto" }) + assert.equals(1, #saved_items()) + end) + + it("does not save on leave when only debounced saves are enabled", function() + config.setup({ + persist = { + enabled = true, + auto = { enabled = true, restore = false, save = true, save_on_leave = false, debounce_ms = 20 }, + }, + }) + auto.setup() + local winid = vim.api.nvim_get_current_win() + push_popup(winid) + wait_for_saved_items(1) + + push_popup(winid) + vim.api.nvim_exec_autocmds("VimLeavePre", { group = "PeekstackPersistAuto" }) + assert.equals(1, #saved_items()) + end) + end) end) diff --git a/tests/persist_lock_spec.lua b/tests/persist_lock_spec.lua new file mode 100644 index 0000000..3784dee --- /dev/null +++ b/tests/persist_lock_spec.lua @@ -0,0 +1,232 @@ +describe("peekstack.persist.lock", function() + local config = require("peekstack.config") + local lock = require("peekstack.persist.lock") + local persist = require("peekstack.persist") + local store = require("peekstack.persist.store") + + local dir = nil + local path = nil + local original_notify = nil + + ---@return PeekstackSession + local function empty_session() + return { items = {}, meta = { created_at = 1, updated_at = 1 } } + end + + ---@return table + local function stored_sessions() + local data = store.read_sync(path) + return data and data.sessions or {} + end + + ---@return integer pid of a process that has exited + local function exited_pid() + local proc = vim.system({ vim.v.progpath, "--clean", "--headless", "+qa!" }) + proc:wait(5000) + return proc.pid + end + + ---Create the lock file as another process holding it would. + ---@param opts? { pid?: integer|false, age_s?: integer } `pid = false` leaves the owner unknown + local function hold_lock_elsewhere(opts) + opts = opts or {} + local lock_path = lock.lock_path(path) + local pid = opts.pid + if pid == nil then + -- A process that keeps running for the whole test. + pid = vim.uv.os_getppid() + end + vim.fn.writefile(pid and { tostring(pid) } or {}, lock_path) + local age_s = opts.age_s + if age_s then + local mtime = os.time() - age_s + assert.is_truthy(vim.uv.fs_utime(lock_path, mtime, mtime)) + end + end + + ---@param name string + ---@param opts? { sync?: boolean } + ---@return fun(): boolean? result + local function save(name, opts) + local result = nil + persist.save_current(name, { + store_path = path, + silent = true, + sync = opts and opts.sync or false, + on_done = function(success) + result = success + end, + }) + return function() + return result + end + end + + before_each(function() + config.setup({ persist = { enabled = true } }) + persist._reset_cache() + dir = vim.fn.tempname() + vim.fn.mkdir(dir, "p") + path = dir .. "/store.json" + original_notify = vim.notify + end) + + after_each(function() + vim.notify = original_notify + persist._reset_cache() + vim.fn.delete(dir, "rf") + end) + + it("waits for another process to release the lock before reading the store", function() + hold_lock_elsewhere() + local result = save("mine") + + vim.wait(60, function() + return false + end, 10) + assert.is_nil(result()) + + -- The other process writes its session, then releases the lock. + assert.is_true(store.write_sync(path, { version = 2, sessions = { theirs = empty_session() } })) + vim.fn.delete(lock.lock_path(path)) + + assert.is_true(vim.wait(3000, function() + return result() ~= nil + end, 10)) + assert.is_true(result()) + local sessions = stored_sessions() + assert.is_not_nil(sessions.mine) + assert.is_not_nil(sessions.theirs) + assert.equals(0, vim.fn.filereadable(lock.lock_path(path))) + end) + + it("takes over a lock left behind by a process that exited", function() + hold_lock_elsewhere({ pid = exited_pid() }) + local result = save("after_exit", { sync = true }) + + assert.is_true(result()) + assert.is_not_nil(stored_sessions().after_exit) + assert.equals(0, vim.fn.filereadable(lock.lock_path(path))) + end) + + it("takes over an old lock whose owner is unknown", function() + hold_lock_elsewhere({ pid = false, age_s = 60 }) + local result = save("after_unknown", { sync = true }) + + assert.is_true(result()) + assert.is_not_nil(stored_sessions().after_unknown) + end) + + it("fails without writing while a running process keeps the lock, however old", function() + local warnings = {} + vim.notify = function(msg) + warnings[#warnings + 1] = msg + end + hold_lock_elsewhere({ age_s = 60 }) + local result = save("blocked", { sync = true }) + + assert.is_false(result()) + assert.equals(0, vim.fn.filereadable(path)) + assert.equals(1, vim.fn.filereadable(lock.lock_path(path))) + assert.is_truthy(warnings[1] and warnings[1]:find("Timed out waiting for another Neovim", 1, true)) + end) + + it("leaves a lock alone that replaced the abandoned one it found", function() + hold_lock_elsewhere({ pid = exited_pid() }) + local lock_path = lock.lock_path(path) + local guard_path = lock_path .. ".takeover" + local original_open = vim.uv.fs_open + local replaced = false + -- Another instance takes the abandoned lock over and acquires a new one + -- between this instance's check and its own takeover. + vim.uv.fs_open = function(file, ...) + if file == guard_path and not replaced then + replaced = true + vim.fn.delete(lock_path) + vim.fn.writefile({ tostring(vim.uv.os_getppid()) }, lock_path) + local future = os.time() + 5 + vim.uv.fs_utime(lock_path, future, future) + end + return original_open(file, ...) + end + vim.notify = function() end + local ok, err = pcall(function() + local result = save("raced", { sync = true }) + assert.is_false(result()) + end) + vim.uv.fs_open = original_open + if not ok then + error(err) + end + + assert.is_true(replaced) + assert.equals(tostring(vim.uv.os_getppid()), vim.fn.readfile(lock_path)[1]) + assert.equals(0, vim.fn.filereadable(guard_path)) + assert.is_nil(stored_sessions().raced) + end) + + it("removes a takeover guard left behind by a process that exited", function() + hold_lock_elsewhere({ pid = exited_pid() }) + local guard_path = lock.lock_path(path) .. ".takeover" + vim.fn.writefile({}, guard_path) + local old = os.time() - 60 + assert.is_truthy(vim.uv.fs_utime(guard_path, old, old)) + + local result = save("after_guard", { sync = true }) + assert.is_true(result()) + assert.is_not_nil(stored_sessions().after_guard) + assert.equals(0, vim.fn.filereadable(guard_path)) + end) + + it("keeps every session saved concurrently by separate Neovim processes", function() + local count = 30 + local start_file = dir .. "/start" + local script = dir .. "/save_sessions.lua" + vim.fn.writefile({ + "local root, store_path, prefix, count, start_file = arg[1], arg[2], arg[3], tonumber(arg[4]), arg[5]", + "vim.opt.runtimepath:append(root)", + "require('peekstack.config').setup({ persist = { enabled = true } })", + "local orchestrator = require('peekstack.persist.orchestrator')", + "local sessions = require('peekstack.persist.sessions')", + "while not vim.uv.fs_stat(start_file) do vim.uv.sleep(5) end", + "for i = 1, count do", + " local ok = orchestrator.update_sync(store_path, function(data)", + " sessions.upsert(data, prefix .. i, {})", + " return true", + " end)", + " if not ok then os.exit(1) end", + " vim.uv.sleep(2)", + "end", + }, script) + + local root = vim.fn.getcwd() + local procs = {} + for _, prefix in ipairs({ "first_", "second_" }) do + procs[#procs + 1] = vim.system({ + vim.v.progpath, + "--headless", + "--clean", + "-l", + script, + root, + path, + prefix, + tostring(count), + start_file, + }, { text = true }) + end + vim.fn.writefile({}, start_file) + + for _, proc in ipairs(procs) do + local done = proc:wait(10000) + assert.equals(0, done.code, done.stderr) + end + + local sessions = stored_sessions() + for _, prefix in ipairs({ "first_", "second_" }) do + for i = 1, count do + assert.is_not_nil(sessions[prefix .. i], "missing session " .. prefix .. i) + end + end + end) +end) diff --git a/tests/persist_sessions_spec.lua b/tests/persist_sessions_spec.lua index c11b921..6715423 100644 --- a/tests/persist_sessions_spec.lua +++ b/tests/persist_sessions_spec.lua @@ -1081,9 +1081,41 @@ describe("peekstack.persist.sessions", function() local items = data.sessions.pinned_session.items assert.equals(2, #items) assert.is_true(items[1].pinned) + assert.equals("copy", items[1].buffer_mode) assert.equals("source", items[2].buffer_mode) end) + for _, default_mode in ipairs({ "copy", "source" }) do + it("should restore each popup's buffer mode under a " .. default_mode .. " default", function() + config.setup({ persist = { enabled = true }, ui = { popup = { buffer_mode = default_mode } } }) + local path = make_file("restore_mode_" .. default_mode, { "line1", "line2" }) + assert.is_not_nil(stack.push(make_location(path, 0), { title = "Copy", buffer_mode = "copy" })) + assert.is_not_nil(stack.push(make_location(path, 1), { title = "Source", buffer_mode = "source" })) + persist.save_current("buffer_modes", { silent = true, sync = true }) + cleanup_stack() + + local restored = nil + persist.restore("buffer_modes", { + silent = true, + on_done = function(result) + restored = result + end, + }) + local waited = vim.wait(wait_timeout_ms, function() + return restored ~= nil + end, wait_interval_ms) + assert.is_true(waited, "Timed out waiting for restore callback") + assert.is_true(restored) + + local popups = stack.list() + assert.equals(2, #popups) + assert.equals("copy", popups[1].buffer_mode) + assert.are_not.equal(vim.fn.bufnr(path), popups[1].bufnr) + assert.equals("source", popups[2].buffer_mode) + assert.equals(vim.fn.bufnr(path), popups[2].bufnr) + end) + end + it("should save parent_popup_id in session items", function() local path = make_file("parent_child", { "line1", "line2" }) local parent = stack.push(make_location(path, 0), { title = "Parent" })