From b535c236a95c3203b62bbe68d43f0b707089e640 Mon Sep 17 00:00:00 2001 From: William Hsieh Date: Mon, 15 Jun 2026 19:30:23 +0800 Subject: [PATCH] refactor(peek): replace adapter registry with push-model peek sources The peek subsystem dispatched through a name-keyed adapter table (default_adapters[name] / Config.adapters[name]) resolved at call time. That registry was stringly-typed, invisible to lua-ls, and existed mainly to serve a custom-adapter-by-name extension surface that isn't a goal (pre-1.0, internal cleanup). The built-in dispatch never needed it -- the names are fixed at author time and the module IS the handle. New shape ("push"): a peek source is just a function that builds OverlookPopupOptions and hands them to require("overlook.window").open_popup. There is no dispatcher, no contract object, no .async flag, no registry, no name lookup. Sync sources call open_popup directly; async sources (LSP) call it later from their own callback -- the sync/async distinction disappears. - Delete lua/overlook/peek.lua (the metatable dispatcher). - lua/overlook/adapter/ -> lua/overlook/peek/ : cursor, marks, definition are now plain functions. require("overlook.peek.cursor")() reads as the action. - window.lua: add module-level M.open_popup(opts) = M.current():open_popup(opts) so a source never has to know the Window/Stack/Popup layering. The seam lives where popups already live rather than in a separate peek layer. - api.lua: peek_* wrappers call the source modules directly; drop the Peek require. - config.lua: remove the `adapters` option (and the leaked your_custom_adapter stub) -- custom sources are now "write a function that calls Window.open_popup and bind it to a key", no registration. - types.lua: drop the OverlookAdapter contract; document a peek source as a plain function (OverlookPeekSource alias). Folds in the bug fixes from the earlier cleanup pass (unpushed, superseded): - peek_mark no longer fires the stray pre-prompt marks() call (double error). - definition guards empty/nil LSP results + missing user_data (was a crash). - cursor drops the dead file_path field. - a source that has nothing to peek notifies and simply does not open; no generic "returned nil options" error. Tests: per-source specs (cursor/marks/definition) assert the source calls Window.open_popup with the right options (or notifies and opens nothing); api_spec covers peek_mark's prompt flow; the dispatcher spec is removed. 99 pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- lua/overlook/adapter/cursor.lua | 28 ----- lua/overlook/adapter/definition.lua | 29 ----- lua/overlook/adapter/marks.lua | 53 --------- lua/overlook/api.lua | 8 +- lua/overlook/config.lua | 19 +-- lua/overlook/peek.lua | 72 ------------ lua/overlook/peek/cursor.lua | 21 ++++ lua/overlook/peek/definition.lua | 32 ++++++ lua/overlook/peek/marks.lua | 42 +++++++ lua/overlook/popup.lua | 2 +- lua/overlook/types.lua | 9 ++ lua/overlook/window.lua | 9 ++ tests/spec/api_spec.lua | 80 +++++++++++++ tests/spec/cursor_spec.lua | 172 +++++++++------------------- tests/spec/definition_spec.lua | 89 ++++++++++++++ tests/spec/marks_spec.lua | 136 +++++++++++----------- tests/spec/peek_spec.lua | 122 -------------------- 17 files changed, 409 insertions(+), 514 deletions(-) delete mode 100644 lua/overlook/adapter/cursor.lua delete mode 100644 lua/overlook/adapter/definition.lua delete mode 100644 lua/overlook/adapter/marks.lua delete mode 100644 lua/overlook/peek.lua create mode 100644 lua/overlook/peek/cursor.lua create mode 100644 lua/overlook/peek/definition.lua create mode 100644 lua/overlook/peek/marks.lua create mode 100644 tests/spec/api_spec.lua create mode 100644 tests/spec/definition_spec.lua delete mode 100644 tests/spec/peek_spec.lua diff --git a/lua/overlook/adapter/cursor.lua b/lua/overlook/adapter/cursor.lua deleted file mode 100644 index 2f68d29..0000000 --- a/lua/overlook/adapter/cursor.lua +++ /dev/null @@ -1,28 +0,0 @@ -local M = {} - ---- Get options for a cursor peek popup. ----@return OverlookPopupOptions? opts Table suitable for Window:open_popup, or nil on error. -function M.get() - local buf = vim.api.nvim_get_current_buf() - local file_path = vim.api.nvim_buf_get_name(buf) - if file_path == "" then - vim.notify("Overlook: Cannot peek in unnamed buffer.", vim.log.levels.WARN) - return nil - end - - ---@diagnostic disable-next-line: unused-local - local bufnum, lnum, col, _off = unpack(vim.fn.getpos(".")) - - local filepath = vim.api.nvim_buf_get_name(bufnum) - local display_path = vim.fn.fnamemodify(filepath, ":~:.") -- Title content - - return { - title = display_path, - target_bufnr = vim.api.nvim_get_current_buf(), - file_path = file_path, - lnum = lnum, - col = col, - } -end - -return M diff --git a/lua/overlook/adapter/definition.lua b/lua/overlook/adapter/definition.lua deleted file mode 100644 index 7ab35f9..0000000 --- a/lua/overlook/adapter/definition.lua +++ /dev/null @@ -1,29 +0,0 @@ -local M = {} - -M.async = true - ----@param location_opts? vim.lsp.LocationOpts -function M.async_create_popup(create_popup_callback, location_opts) - location_opts = location_opts or {} - - vim.lsp.buf.definition { - on_list = function(tt) - -- vim.print("LSP Definition Locations: ", vim.inspect(tt)) - local item = tt.items[1] - local uri = item.user_data.targetUri or item.user_data.uri - if not uri then - vim.notify("Overlook: No URI found in LSP definition item: " .. vim.inspect(tt), vim.log.levels.WARN) - return - end - - create_popup_callback { - target_bufnr = vim.uri_to_bufnr(uri), - lnum = item.lnum, - col = item.col, - title = item.filename, - } - end, - } -end - -return M diff --git a/lua/overlook/adapter/marks.lua b/lua/overlook/adapter/marks.lua deleted file mode 100644 index 2aa6154..0000000 --- a/lua/overlook/adapter/marks.lua +++ /dev/null @@ -1,53 +0,0 @@ -local api = vim.api -local M = {} - ----Gets the options required for the peek popup for a specific mark. ----Returns nil if the mark is invalid or an error occurs. ----@param mark_char string ----@return OverlookPopupOptions? opts Table suitable for Window:open_popup, or nil on error. -function M.get(mark_char) - -- Input validation - if not mark_char or #mark_char ~= 1 then - vim.notify("Overlook Error: Invalid mark character provided.", vim.log.levels.ERROR) - return nil -- Return nil on invalid input - end - - local pos = vim.fn.getpos("'" .. mark_char) - - local bufnum = pos[1] - local lnum = pos[2] - local col = pos[3] - - if bufnum == 0 or lnum == 0 then - vim.notify("Overlook: Mark '" .. mark_char .. "' is not set.", vim.log.levels.INFO) - return nil -- Return nil if mark not set - end - - -- Basic validation checks - if not api.nvim_buf_is_loaded(bufnum) then - -- Consider adding an option to load the buffer if desired. - vim.notify("Overlook Info: Buffer for mark '" .. mark_char .. "' is not loaded.", vim.log.levels.INFO) - return nil - end - if not api.nvim_buf_is_valid(bufnum) then - vim.notify( - "Overlook Error: Buffer for mark '" .. mark_char .. "' (" .. bufnum .. ") is invalid.", - vim.log.levels.ERROR - ) - return nil -- Return nil if buffer invalid - end - - local filepath = api.nvim_buf_get_name(bufnum) - local display_path = vim.fn.fnamemodify(filepath, ":~:.") -- Title content - - -- Return the options table for the popup - return { - target_bufnr = bufnum, - lnum = lnum, - col = col, - title = display_path, - -- Add any other mark-specific options here if needed in the future - } -end - -return M diff --git a/lua/overlook/api.lua b/lua/overlook/api.lua index 3b265a6..8ce20b9 100644 --- a/lua/overlook/api.lua +++ b/lua/overlook/api.lua @@ -9,7 +9,6 @@ --- ---@tag overlook-api -local Peek = require("overlook.peek") local Window = require("overlook.window") local M = {} @@ -36,7 +35,7 @@ local M = {} ---@tag overlook-api.peek_definition ---@toc_entry M.peek_definition = function() - Peek.definition() + require("overlook.peek.definition")() end --- Switch focus between the top popup and the root window. @@ -66,7 +65,7 @@ end ---@tag overlook-api.peek_cursor ---@toc_entry M.peek_cursor = function() - Peek.cursor() + require("overlook.peek.cursor")() end --- Peek at a specific mark location. @@ -83,14 +82,13 @@ end ---@tag overlook-api.peek_mark ---@toc_entry M.peek_mark = function() - Peek.marks() vim.ui.input({ prompt = "Overlook Mark:" }, function(input) if input == nil or input == "" then return end if #input == 1 then - Peek.marks(input) + require("overlook.peek.marks")(input) else vim.notify("Overlook Error: Invalid mark. Please enter a single character.", vim.log.levels.ERROR) end diff --git a/lua/overlook/config.lua b/lua/overlook/config.lua index f323fff..6464f5b 100644 --- a/lua/overlook/config.lua +++ b/lua/overlook/config.lua @@ -67,19 +67,11 @@ local M = {} ---@field size_ratio number Default size ratio (0.0 to 1.0) used to calculate initial size. ---@field keys? table Keymaps specific to the popup UI. ---- *OverlookAdapterOptions* ---- ----@class OverlookAdapterOptions ---- ----@field marks? table Configuration for the 'marks' adapter. --- ---@field lsp? table Placeholder for future LSP adapter config - --- *OverlookOptions* --- ---@class OverlookOptions --- ---@field ui OverlookOptions.UI UI settings for the popup windows. ----@field adapters OverlookAdapterOptions Adapter-specific configurations. ---@field on_stack_empty? fun() Optional function called when the last Overlook popup closes. --- Default configuration options for overlook.nvim. @@ -132,15 +124,6 @@ local defaults = { }, }, - -- Adapter-specific configurations - adapters = { - -- check `overlook.adapter.cursor` for implementation details - your_custom_adapter = { - ---@return OverlookPopupOptions? @Table suitable for Window:open_popup, or nil on error. - get = function() end, - }, - }, - -- Optional hook called when the last Overlook popup closes on_stack_empty = nil, } @@ -154,7 +137,7 @@ local options = vim.deepcopy(defaults) ---@param user_opts? table User configuration options. Can contain any subset of OverlookOptions fields. function M.setup(user_opts) if user_opts then - -- Use deep_extend to merge nested tables like 'ui' and 'adapters'. + -- Use deep_extend to merge nested tables like 'ui'. -- 'force' mode replaces arrays entirely if present, usually desired for config. options = vim.tbl_deep_extend("force", options, user_opts or {}) end diff --git a/lua/overlook/peek.lua b/lua/overlook/peek.lua deleted file mode 100644 index 4e61e20..0000000 --- a/lua/overlook/peek.lua +++ /dev/null @@ -1,72 +0,0 @@ -local Config = require("overlook.config") - -local M = {} - -local default_adapters = { - marks = require("overlook.adapter.marks"), - definition = require("overlook.adapter.definition"), - cursor = require("overlook.adapter.cursor"), -} - -local get_adapter_if_valid = function(adapter) - if adapter == nil then - return nil - end - - if adapter.async then - if not adapter.async_create_popup then - return nil - end - else - -- TODO: rename get to sync_get_popup_options - if not (type(adapter.get) == "function") then - return nil - end - end - - return adapter -end - ---- Generic peek function that calls the appropriate adapter's get() method ---- @param adapter_type string The type of adapter ('marks', 'definition', etc.) ---- @param ... any Arguments to pass to the adapter's get() function -local function peek_with_adapters(adapter_type, ...) - local adapter = get_adapter_if_valid(Config.get().adapters[adapter_type]) - or get_adapter_if_valid(default_adapters[adapter_type]) - - if not adapter then - vim.notify( - "Overlook Error: Invalid adapter type or adapter missing get() function: " .. adapter_type, - vim.log.levels.ERROR - ) - return - end - - -- async adapters - if adapter.async then - adapter.async_create_popup(function(opts) - require("overlook.window").current():open_popup(opts) - end, ...) - return - end - - -- synchronous adapters - ---@type OverlookPopupOptions? - local opts = adapter.get(...) - if not opts then - vim.notify("Overlook Error: Adapter '" .. adapter_type .. "' returned nil options.", vim.log.levels.ERROR) - return - end - - require("overlook.window").current():open_popup(opts) -end - -setmetatable(M, { - __index = function(_, key) - return function(...) - return peek_with_adapters(key, ...) - end - end, -}) - -return M diff --git a/lua/overlook/peek/cursor.lua b/lua/overlook/peek/cursor.lua new file mode 100644 index 0000000..9ce62a5 --- /dev/null +++ b/lua/overlook/peek/cursor.lua @@ -0,0 +1,21 @@ +--- Peek at the current cursor position: opens a popup of the current buffer +--- anchored at the cursor. A peek source is just a function that builds +--- OverlookPopupOptions and hands them to the window layer. +---@return nil +return function() + local file_path = vim.api.nvim_buf_get_name(0) + if file_path == "" then + vim.notify("Overlook: Cannot peek in unnamed buffer.", vim.log.levels.WARN) + return + end + + ---@diagnostic disable-next-line: unused-local + local _bufnum, lnum, col, _off = unpack(vim.fn.getpos(".")) + + require("overlook.window").open_popup { + title = vim.fn.fnamemodify(file_path, ":~:."), + target_bufnr = vim.api.nvim_get_current_buf(), + lnum = lnum, + col = col, + } +end diff --git a/lua/overlook/peek/definition.lua b/lua/overlook/peek/definition.lua new file mode 100644 index 0000000..b7ff8d5 --- /dev/null +++ b/lua/overlook/peek/definition.lua @@ -0,0 +1,32 @@ +--- Peek at the LSP definition under the cursor. Asynchronous: the popup opens +--- from the LSP on_list callback once the server responds. Guards the common +--- "no definition" / malformed-result cases and simply does not open a popup +--- in those cases. +---@param location_opts? vim.lsp.LocationOpts +---@return nil +return function(location_opts) + location_opts = location_opts or {} + + vim.lsp.buf.definition { + on_list = function(tt) + local item = tt and tt.items and tt.items[1] + if not item then + vim.notify("Overlook: No definition found.", vim.log.levels.INFO) + return + end + + local uri = item.user_data and (item.user_data.targetUri or item.user_data.uri) + if not uri then + vim.notify("Overlook: No URI found in LSP definition item.", vim.log.levels.WARN) + return + end + + require("overlook.window").open_popup { + target_bufnr = vim.uri_to_bufnr(uri), + lnum = item.lnum, + col = item.col, + title = item.filename, + } + end, + } +end diff --git a/lua/overlook/peek/marks.lua b/lua/overlook/peek/marks.lua new file mode 100644 index 0000000..b151d8b --- /dev/null +++ b/lua/overlook/peek/marks.lua @@ -0,0 +1,42 @@ +local api = vim.api + +--- Peek at a mark's location. Validates the mark, then opens a popup of its +--- buffer at the mark position. Notifies and returns early on any invalid / +--- unset / unloaded case (it never opens a popup in those cases). +---@param mark_char string Single-character mark name. +---@return nil +return function(mark_char) + if not mark_char or #mark_char ~= 1 then + vim.notify("Overlook Error: Invalid mark character provided.", vim.log.levels.ERROR) + return + end + + local pos = vim.fn.getpos("'" .. mark_char) + local bufnum = pos[1] + local lnum = pos[2] + local col = pos[3] + + if bufnum == 0 or lnum == 0 then + vim.notify("Overlook: Mark '" .. mark_char .. "' is not set.", vim.log.levels.INFO) + return + end + + if not api.nvim_buf_is_loaded(bufnum) then + vim.notify("Overlook Info: Buffer for mark '" .. mark_char .. "' is not loaded.", vim.log.levels.INFO) + return + end + if not api.nvim_buf_is_valid(bufnum) then + vim.notify( + "Overlook Error: Buffer for mark '" .. mark_char .. "' (" .. bufnum .. ") is invalid.", + vim.log.levels.ERROR + ) + return + end + + require("overlook.window").open_popup { + target_bufnr = bufnum, + lnum = lnum, + col = col, + title = vim.fn.fnamemodify(api.nvim_buf_get_name(bufnum), ":~:."), + } +end diff --git a/lua/overlook/popup.lua b/lua/overlook/popup.lua index 287f6a9..bd1191e 100644 --- a/lua/overlook/popup.lua +++ b/lua/overlook/popup.lua @@ -211,7 +211,7 @@ function Popup:snapshot_state() self.opts.target_bufnr = api.nvim_win_get_buf(self.winid) local cursor = api.nvim_win_get_cursor(self.winid) self.opts.lnum = cursor[1] - -- opts.col is 1-indexed (the adapters use vim.fn.getpos which returns + -- opts.col is 1-indexed (the peek sources use vim.fn.getpos which returns -- 1-indexed); nvim_win_get_cursor returns 0-indexed. Convert. -- set_cursor_position converts back via math.max(0, col - 1) on restore. self.opts.col = cursor[2] + 1 diff --git a/lua/overlook/types.lua b/lua/overlook/types.lua index a0a6a69..4190025 100644 --- a/lua/overlook/types.lua +++ b/lua/overlook/types.lua @@ -3,3 +3,12 @@ ---@field lnum integer The line number to position the popup. ---@field col integer The column number to position the popup. ---@field title? string Optional title for the popup. + +--- A peek source is just a function. It gathers some context (the cursor, a +--- mark, an LSP result, ...), builds OverlookPopupOptions, and hands them to +--- `require("overlook.window").open_popup(opts)`. It opens nothing (and +--- typically notifies) when there is nothing to peek. Async sources (e.g. +--- vim.lsp.buf.*) simply call open_popup later from their own callback. The +--- built-in sources live under `overlook.peek.*`; a custom source is any +--- function following the same shape, bound to a key. +---@alias OverlookPeekSource fun(...): nil diff --git a/lua/overlook/window.lua b/lua/overlook/window.lua index 2def704..21e32f8 100644 --- a/lua/overlook/window.lua +++ b/lua/overlook/window.lua @@ -43,6 +43,15 @@ function M.current() return M.get(winid) end +---Open a popup in the current context's window stack. The single entry point +---peek sources use: they build OverlookPopupOptions and hand them here, so a +---source never has to know about the Window/Stack/Popup layering. +---@param opts OverlookPopupOptions +---@return OverlookPopup? +function M.open_popup(opts) + return M.current():open_popup(opts) +end + ---Construct + open a popup using this Window's current context (host, top ---of stack as prev, current depth). Returns the popup on success, nil on ---failure. Shared between open_popup (push semantics) and restore_one diff --git a/tests/spec/api_spec.lua b/tests/spec/api_spec.lua new file mode 100644 index 0000000..72705a0 --- /dev/null +++ b/tests/spec/api_spec.lua @@ -0,0 +1,80 @@ +describe("overlook.api peek_mark", function() + local api_mod + local marks_calls + local notify_calls + local original_notify + local original_ui_input + local ui_input_cb + + before_each(function() + marks_calls = {} + notify_calls = {} + + original_notify = vim.notify + vim.notify = function(msg, level) + table.insert(notify_calls, { msg = msg, level = level }) + end + + -- peek_mark lazily requires the marks source; stub it so we observe calls. + package.loaded["overlook.peek.marks"] = function(...) + table.insert(marks_calls, { ... }) + end + package.loaded["overlook.window"] = { + current = function() + return {} + end, + } + + -- Capture the vim.ui.input callback instead of actually prompting. + original_ui_input = vim.ui.input + ui_input_cb = nil + vim.ui.input = function(_opts, cb) + ui_input_cb = cb + end + + package.loaded["overlook.api"] = nil + api_mod = require("overlook.api") + end) + + after_each(function() + vim.notify = original_notify + vim.ui.input = original_ui_input + package.loaded["overlook.peek.marks"] = nil + package.loaded["overlook.window"] = nil + package.loaded["overlook.api"] = nil + end) + + -- Regression: peek_mark used to call Peek.marks() with no argument BEFORE + -- prompting, which fired marks.get(nil) -> "Invalid mark character" plus a + -- second "returned nil options" ERROR on every invocation. + it("does not invoke the marks adapter (or notify) before a char is entered", function() + api_mod.peek_mark() + assert.are.equal(0, #marks_calls) + assert.are.equal(0, #notify_calls) + end) + + it("invokes marks exactly once with the entered char", function() + api_mod.peek_mark() + ui_input_cb("a") + assert.are.equal(1, #marks_calls) + assert.are.same({ "a" }, marks_calls[1]) + assert.are.equal(0, #notify_calls) + end) + + it("notifies and does not peek on multi-character input", function() + api_mod.peek_mark() + ui_input_cb("ab") + assert.are.equal(0, #marks_calls) + assert.are.equal(1, #notify_calls) + assert.matches("Invalid mark", notify_calls[1].msg) + end) + + it("does nothing on cancelled / empty input", function() + api_mod.peek_mark() + ui_input_cb(nil) + api_mod.peek_mark() + ui_input_cb("") + assert.are.equal(0, #marks_calls) + assert.are.equal(0, #notify_calls) + end) +end) diff --git a/tests/spec/cursor_spec.lua b/tests/spec/cursor_spec.lua index 5534041..51953d3 100644 --- a/tests/spec/cursor_spec.lua +++ b/tests/spec/cursor_spec.lua @@ -1,154 +1,94 @@ -describe("Cursor Adapter", function() - local peek_mod - +describe("overlook.peek.cursor", function() + local cursor + local open_popup_calls + local notify_calls local original_notify - local mock_calls - local original_get_current_buf -- Store original vim.api.nvim_get_current_buf - local original_getpos -- Store original vim.fn.getpos + local original_get_current_buf + local original_getpos + local original_buf_get_name before_each(function() - -- Reset mock calls table - mock_calls = { - open_popup = {}, - notify = {}, - } + open_popup_calls = {} + notify_calls = {} - -- Stub overlook.window so peek doesn't need a real Neovim window - local fake_window = { - open_popup = function(_, opts) - table.insert(mock_calls.open_popup, opts) - end, - } + -- The source hands options to require("overlook.window").open_popup. package.loaded["overlook.window"] = { - current = function() - return fake_window + open_popup = function(opts) + table.insert(open_popup_calls, opts) end, } - -- Store and mock vim.notify original_notify = vim.notify - vim.notify = function(msg, level, opts) - table.insert(mock_calls.notify, { msg = msg, level = level, opts = opts }) + vim.notify = function(msg, level) + table.insert(notify_calls, { msg = msg, level = level }) end - -- Store and mock vim.api.nvim_get_current_buf original_get_current_buf = vim.api.nvim_get_current_buf - vim.api.nvim_get_current_buf = function() - -- This will be overridden in the specific test that creates a buffer - return 0 -- Default mock for tests that don't care or handle unnamed - end - - -- Store and mock vim.fn.getpos original_getpos = vim.fn.getpos - vim.fn.getpos = function(target) - if target == "." then - -- This will be overridden in the specific test that sets cursor - return { 0, 1, 1, 0 } -- Default mock - end - return original_getpos(target) -- Call original for other targets - end + original_buf_get_name = vim.api.nvim_buf_get_name - -- Reload peek so it picks up the window stub - package.loaded["overlook.peek"] = nil - peek_mod = require("overlook.peek") + package.loaded["overlook.peek.cursor"] = nil + cursor = require("overlook.peek.cursor") end) after_each(function() - -- Restore original functions - if original_notify then - vim.notify = original_notify - end - original_notify = nil - - if original_get_current_buf then - vim.api.nvim_get_current_buf = original_get_current_buf - end - original_get_current_buf = nil - - if original_getpos then - vim.fn.getpos = original_getpos - end - original_getpos = nil - - -- Clean up stubs + vim.notify = original_notify + vim.api.nvim_get_current_buf = original_get_current_buf + vim.fn.getpos = original_getpos + vim.api.nvim_buf_get_name = original_buf_get_name package.loaded["overlook.window"] = nil - package.loaded["overlook.peek"] = nil - - -- Clean up any test buffers if needed + package.loaded["overlook.peek.cursor"] = nil pcall(vim.cmd, "bw! test_buffer.txt") - pcall(vim.cmd, "bw!") -- For unnamed buffer test + pcall(vim.cmd, "bw!") end) - it("should call create_popup with cursor context when peek('cursor') is called", function() - -- Setup: Create a dummy buffer and set content + it("opens a popup with cursor context for a named buffer", function() vim.cmd("edit! test_buffer.txt") - local expected_bufnr = vim.api.nvim_get_current_buf() -- Get the buffer number - -- Override mocks for this specific test case + local bufnr = vim.api.nvim_get_current_buf() + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { "line 1", "line 2", "line 3", "line 4", "line 5" }) + vim.api.nvim_get_current_buf = function() - return expected_bufnr + return bufnr end vim.fn.getpos = function(target) if target == "." then - return { expected_bufnr, 3, 5, 0 } + return { bufnr, 3, 5, 0 } end - return { 0, 0, 0, 0 } -- Should not happen in this test flow for '.' + return { 0, 0, 0, 0 } + end + vim.api.nvim_buf_get_name = function(b) + if b == 0 or b == bufnr then + return "/tmp/test_buffer.txt" + end + return original_buf_get_name(b) end - vim.api.nvim_buf_set_lines(expected_bufnr, 0, -1, false, { - "line 1", - "line 2", - "line 3 is the cursor line", - "line 4", - "line 5", - }) - vim.api.nvim_win_set_cursor(0, { 3, 5 }) -- Set cursor to line 3, column 5 - - -- Action - peek_mod.cursor() - - -- Assert - assert.are.equal(1, #mock_calls.open_popup) - local call_args = mock_calls.open_popup[1] - assert.is_table(call_args) - assert.matches("test_buffer.txt", call_args.title) -- Expect filename again - assert.are.equal(expected_bufnr, call_args.target_bufnr) -- target_bufnr should be the actual buffer - assert.are.equal(3, call_args.lnum) - assert.are.equal(5, call_args.col) -- Column from getpos is 1-indexed, set_cursor is 0-indexed - assert.matches("test_buffer.txt", call_args.file_path) -- Check file_path - assert.is_nil(call_args.content) -- Should not have content - assert.is_nil(call_args.highlight_line) -- Should not have highlight_line + cursor() - -- Teardown (moved to after_each) - -- vim.cmd("bw! test_buffer.txt") + assert.are.equal(1, #open_popup_calls) + local o = open_popup_calls[1] + assert.are.equal(bufnr, o.target_bufnr) + assert.are.equal(3, o.lnum) + assert.are.equal(5, o.col) + assert.matches("test_buffer.txt", o.title) + assert.is_nil(o.file_path) -- file_path was removed from the options shape + assert.are.equal(0, #notify_calls) end) - it("should handle unnamed buffers gracefully", function() - -- Setup: Create an unnamed buffer + it("notifies and does not open for an unnamed buffer", function() vim.cmd("enew") - -- Override mocks for this specific test case (unnamed buffer) - vim.api.nvim_get_current_buf = function() - return vim.fn.bufnr() - end -- Use actual current unnamed buf - vim.fn.getpos = function(target) -- Mock getpos for unnamed buffer scenario - if target == "." then - return { vim.fn.bufnr(), 1, 0, 0 } - end -- e.g. line 1, col 0 - return { 0, 0, 0, 0 } + vim.api.nvim_buf_get_name = function(b) + if b == 0 then + return "" + end + return original_buf_get_name(b) end - assert.equal("", vim.api.nvim_buf_get_name(0)) - - -- Action - peek_mod.cursor() - - -- Assert - assert.are.equal(0, #mock_calls.open_popup) -- Should not be called - assert.are.equal(2, #mock_calls.notify) - local notify_call = mock_calls.notify[1] - assert.matches("Cannot peek in unnamed buffer", notify_call.msg) -- Use matches for flexibility - assert.are.equal(vim.log.levels.WARN, notify_call.level) + cursor() - -- Teardown (moved to after_each) - -- vim.cmd("bw!") + assert.are.equal(0, #open_popup_calls) + assert.are.equal(1, #notify_calls) + assert.matches("Cannot peek in unnamed buffer", notify_calls[1].msg) + assert.are.equal(vim.log.levels.WARN, notify_calls[1].level) end) end) diff --git a/tests/spec/definition_spec.lua b/tests/spec/definition_spec.lua new file mode 100644 index 0000000..67f6dd7 --- /dev/null +++ b/tests/spec/definition_spec.lua @@ -0,0 +1,89 @@ +describe("overlook.peek.definition", function() + local definition + local open_popup_calls + local notify_calls + local original_notify + local original_lsp_definition + local original_uri_to_bufnr + + before_each(function() + open_popup_calls = {} + notify_calls = {} + + package.loaded["overlook.window"] = { + open_popup = function(opts) + table.insert(open_popup_calls, opts) + end, + } + + original_notify = vim.notify + vim.notify = function(msg, level) + table.insert(notify_calls, { msg = msg, level = level }) + end + + original_lsp_definition = vim.lsp.buf.definition + original_uri_to_bufnr = vim.uri_to_bufnr + vim.uri_to_bufnr = function(_uri) + return 4242 -- sentinel so we can assert the mapped buffer + end + + package.loaded["overlook.peek.definition"] = nil + definition = require("overlook.peek.definition") + end) + + after_each(function() + vim.notify = original_notify + vim.lsp.buf.definition = original_lsp_definition + vim.uri_to_bufnr = original_uri_to_bufnr + package.loaded["overlook.window"] = nil + package.loaded["overlook.peek.definition"] = nil + end) + + it("opens a popup from the first LSP result", function() + vim.lsp.buf.definition = function(o) + o.on_list { + items = { + { lnum = 12, col = 7, filename = "target.lua", user_data = { uri = "file:///tmp/target.lua" } }, + }, + } + end + + definition() + + assert.are.equal(1, #open_popup_calls) + local o = open_popup_calls[1] + assert.are.equal(4242, o.target_bufnr) + assert.are.equal(12, o.lnum) + assert.are.equal(7, o.col) + assert.are.equal("target.lua", o.title) + assert.are.equal(0, #notify_calls) + end) + + it("notifies INFO and does not open (no crash) on empty results", function() + vim.lsp.buf.definition = function(o) + o.on_list { items = {} } + end + + local ok = pcall(definition) + + assert.is_true(ok) + assert.are.equal(0, #open_popup_calls) + assert.are.equal(1, #notify_calls) + assert.matches("No definition found", notify_calls[1].msg) + assert.are.equal(vim.log.levels.INFO, notify_calls[1].level) + end) + + it("notifies WARN and does not open when the item has no URI", function() + vim.lsp.buf.definition = function(o) + o.on_list { items = { { lnum = 1, col = 1, filename = "x", user_data = {} } } } + end + + local ok = pcall(definition) + + assert.is_true(ok) + assert.are.equal(0, #open_popup_calls) + assert.are.equal(1, #notify_calls) + assert.matches("No URI", notify_calls[1].msg) + assert.are.equal(vim.log.levels.WARN, notify_calls[1].level) + end) +end) diff --git a/tests/spec/marks_spec.lua b/tests/spec/marks_spec.lua index 43ff866..5ba53fb 100644 --- a/tests/spec/marks_spec.lua +++ b/tests/spec/marks_spec.lua @@ -1,84 +1,87 @@ local api = vim.api -describe("overlook.adapter.marks", function() - local marks_adapter - - -- Mock vim.notify to check messages - local notify_calls = {} - local original_notify = vim.notify - local mock_notify = function(msg, level, opts) - table.insert(notify_calls, { msg = msg, level = level, opts = opts }) - end - - -- Mock buffer functions for specific test cases - local original_buf_is_loaded = api.nvim_buf_is_loaded - local original_buf_is_valid = api.nvim_buf_is_valid - local original_buf_get_name = api.nvim_buf_get_name - local original_getpos = vim.fn.getpos +describe("overlook.peek.marks", function() + local marks + local open_popup_calls + local notify_calls + local original_notify + local original_buf_is_loaded + local original_buf_is_valid + local original_buf_get_name + local original_getpos before_each(function() - -- Reload the module before each test to reset state if necessary - package.loaded["overlook.adapter.marks"] = nil - marks_adapter = require("overlook.adapter.marks") + open_popup_calls = {} notify_calls = {} - vim.notify = mock_notify - -- Reset mocks - api.nvim_buf_is_loaded = original_buf_is_loaded - api.nvim_buf_is_valid = original_buf_is_valid - api.nvim_buf_get_name = original_buf_get_name - vim.fn.getpos = original_getpos + + package.loaded["overlook.window"] = { + open_popup = function(opts) + table.insert(open_popup_calls, opts) + end, + } + + original_notify = vim.notify + vim.notify = function(msg, level) + table.insert(notify_calls, { msg = msg, level = level }) + end + + original_buf_is_loaded = api.nvim_buf_is_loaded + original_buf_is_valid = api.nvim_buf_is_valid + original_buf_get_name = api.nvim_buf_get_name + original_getpos = vim.fn.getpos + + package.loaded["overlook.peek.marks"] = nil + marks = require("overlook.peek.marks") end) after_each(function() - -- Restore original functions vim.notify = original_notify api.nvim_buf_is_loaded = original_buf_is_loaded api.nvim_buf_is_valid = original_buf_is_valid api.nvim_buf_get_name = original_buf_get_name vim.fn.getpos = original_getpos - -- Clean up any marks set? (Potentially needed depending on tests) + package.loaded["overlook.window"] = nil + package.loaded["overlook.peek.marks"] = nil end) - it("should return nil for invalid mark characters", function() - assert.is_nil(marks_adapter.get("")) - assert.are.equal(1, #notify_calls) - assert.matches("Invalid mark character", notify_calls[1].msg) - - notify_calls = {} -- Reset for next check - assert.is_nil(marks_adapter.get("ab")) - assert.are.equal(1, #notify_calls) - assert.matches("Invalid mark character", notify_calls[1].msg) + it("notifies and does not open for invalid mark characters", function() + for _, bad in ipairs { "", "ab" } do + notify_calls = {} + marks(bad) + assert.are.equal(0, #open_popup_calls) + assert.are.equal(1, #notify_calls) + assert.matches("Invalid mark character", notify_calls[1].msg) + end notify_calls = {} - assert.is_nil(marks_adapter.get(nil)) + marks(nil) + assert.are.equal(0, #open_popup_calls) assert.are.equal(1, #notify_calls) assert.matches("Invalid mark character", notify_calls[1].msg) end) - it("should return nil if mark is not set", function() - -- Mock getpos to simulate unset mark 'x' + it("notifies and does not open if the mark is not set", function() vim.fn.getpos = function(mark) if mark == "'x" then - return { 0, 0, 0, 0 } -- Unset position + return { 0, 0, 0, 0 } end return original_getpos(mark) end - assert.is_nil(marks_adapter.get("x")) + marks("x") + assert.are.equal(0, #open_popup_calls) assert.are.equal(1, #notify_calls) assert.matches("Mark 'x' is not set", notify_calls[1].msg) end) - it("should return nil if buffer is not loaded", function() + it("notifies and does not open if the buffer is not loaded", function() local mock_bufnr = 999 - -- Mock getpos to return a valid position but for a specific buffer vim.fn.getpos = function(mark) if mark == "'l" then return { mock_bufnr, 10, 5, 0 } end return original_getpos(mark) end - -- Mock nvim_buf_is_loaded to return false for our mock buffer api.nvim_buf_is_loaded = function(bufnr) if bufnr == mock_bufnr then return false @@ -86,12 +89,13 @@ describe("overlook.adapter.marks", function() return original_buf_is_loaded(bufnr) end - assert.is_nil(marks_adapter.get("l")) + marks("l") + assert.are.equal(0, #open_popup_calls) assert.are.equal(1, #notify_calls) assert.matches("Buffer for mark 'l' is not loaded", notify_calls[1].msg) end) - it("should return nil if buffer is not valid", function() + it("notifies and does not open if the buffer is not valid", function() local mock_bufnr = 998 vim.fn.getpos = function(mark) if mark == "'v" then @@ -102,46 +106,37 @@ describe("overlook.adapter.marks", function() api.nvim_buf_is_loaded = function(bufnr) if bufnr == mock_bufnr then return true - end -- Assume loaded + end return original_buf_is_loaded(bufnr) end api.nvim_buf_is_valid = function(bufnr) if bufnr == mock_bufnr then return false - end -- But invalid + end return original_buf_is_valid(bufnr) end - assert.is_nil(marks_adapter.get("v")) + marks("v") + assert.are.equal(0, #open_popup_calls) assert.are.equal(1, #notify_calls) assert.matches("Buffer for mark 'v' .* is invalid", notify_calls[1].msg) end) - it("should return opts table for a valid mark", function() - -- Setup a valid mark 'a' (e.g., pointing to current buffer, line 5, col 3) + it("opens a popup for a valid mark", function() local current_bufnr = api.nvim_get_current_buf() local current_buf_name = api.nvim_buf_get_name(current_bufnr) - local expected_lnum = 5 - local expected_col = 3 vim.fn.getpos = function(mark) if mark == "'a" then - return { current_bufnr, expected_lnum, expected_col, 0 } + return { current_bufnr, 5, 3, 0 } end return original_getpos(mark) end - -- Ensure buffer is considered loaded and valid for this test api.nvim_buf_is_loaded = function(bufnr) - if bufnr == current_bufnr then - return true - end - return original_buf_is_loaded(bufnr) + return bufnr == current_bufnr or original_buf_is_loaded(bufnr) end api.nvim_buf_is_valid = function(bufnr) - if bufnr == current_bufnr then - return true - end - return original_buf_is_valid(bufnr) + return bufnr == current_bufnr or original_buf_is_valid(bufnr) end api.nvim_buf_get_name = function(bufnr) if bufnr == current_bufnr then @@ -150,13 +145,14 @@ describe("overlook.adapter.marks", function() return original_buf_get_name(bufnr) end - local opts = marks_adapter.get("a") - assert.is_table(opts) - assert.are.equal(current_bufnr, opts.target_bufnr) - assert.are.equal(expected_lnum, opts.lnum) - assert.are.equal(expected_col, opts.col) - assert.is_string(opts.title) - assert.matches(vim.fn.fnamemodify(current_buf_name, ":t"), opts.title) -- Check title contains filename - assert.are.equal(0, #notify_calls) -- No errors expected + marks("a") + + assert.are.equal(1, #open_popup_calls) + local o = open_popup_calls[1] + assert.are.equal(current_bufnr, o.target_bufnr) + assert.are.equal(5, o.lnum) + assert.are.equal(3, o.col) + assert.is_string(o.title) + assert.are.equal(0, #notify_calls) end) end) diff --git a/tests/spec/peek_spec.lua b/tests/spec/peek_spec.lua deleted file mode 100644 index f596d96..0000000 --- a/tests/spec/peek_spec.lua +++ /dev/null @@ -1,122 +0,0 @@ -describe("overlook.peek", function() - local peek_mod - local mock_marks_adapter - local fake_window - local create_popup_calls - local marks_get_calls - - -- Mock vim.notify - local notify_calls = {} - local original_notify = vim.notify - local mock_notify = function(msg, level, opts) - table.insert(notify_calls, { msg = msg, level = level, opts = opts }) - end - - before_each(function() - notify_calls = {} - vim.notify = mock_notify - - -- Mock dependencies - marks_get_calls = {} - mock_marks_adapter = { - get = function(...) - table.insert(marks_get_calls, { ... }) - -- Default mock behavior: return a dummy opts table - return { target_bufnr = 1, lnum = 1, col = 1, title = "mock" } - end, - } - - create_popup_calls = {} - fake_window = { - open_popup = function(_, opts) - table.insert(create_popup_calls, opts) - end, - } - - -- Inject mocks using package.loaded trick - package.loaded["overlook.adapter.marks"] = mock_marks_adapter - package.loaded["overlook.window"] = { - current = function() - return fake_window - end, - } - - -- Reload the peek module to use the mocks - package.loaded["overlook.peek"] = nil - peek_mod = require("overlook.peek") - end) - - after_each(function() - -- Restore originals - vim.notify = original_notify - package.loaded["overlook.adapter.marks"] = nil - package.loaded["overlook.window"] = nil - package.loaded["overlook.peek"] = nil - end) - - it("should call the correct adapter's get method and ui.create_popup", function() - local mark_char = "m" - local expected_opts = { target_bufnr = 123, lnum = 45, col = 6, title = "test_mark" } - - -- Override mock get for this specific test - mock_marks_adapter.get = function(arg) - table.insert(marks_get_calls, { arg }) - if arg == mark_char then - return expected_opts - end - return nil - end - - peek_mod.marks(mark_char) - - -- Check adapter was called correctly - assert.are.equal(1, #marks_get_calls) - assert.are.same({ mark_char }, marks_get_calls[1]) - - -- Check UI was called correctly - assert.are.equal(1, #create_popup_calls) - assert.are.same(expected_opts, create_popup_calls[1]) - - -- Check no errors notified - assert.are.equal(0, #notify_calls) - end) - - it("should notify and return if adapter type is invalid", function() - peek_mod.peek("invalid_adapter", "a") - - assert.are.equal(1, #notify_calls) - assert.matches("Invalid adapter type", notify_calls[1].msg) - assert.are.equal(0, #marks_get_calls) - assert.are.equal(0, #create_popup_calls) - end) - - it("should notify and return if adapter is missing get method", function() - -- Temporarily break the mock adapter - package.loaded["overlook.adapter.marks"] = { not_get = function() end } - package.loaded["overlook.peek"] = nil - peek_mod = require("overlook.peek") - - peek_mod.marks("a") - - assert.are.equal(1, #notify_calls) - assert.matches("Invalid adapter type or adapter missing get().*marks", notify_calls[1].msg) - assert.are.equal(0, #create_popup_calls) - - -- Restore for subsequent tests (important!) - package.loaded["overlook.adapter.marks"] = mock_marks_adapter - end) - - it("should return without calling popup if adapter's get returns nil", function() - -- Mock get to return nil - mock_marks_adapter.get = function(...) - table.insert(marks_get_calls, { ... }) - return nil -- Simulate adapter handling an error/no data - end - - peek_mod.marks("z") - - assert.are.equal(1, #marks_get_calls) -- Adapter get should still be called - assert.are.equal(0, #create_popup_calls) -- UI should NOT be called - assert.are.equal(1, #notify_calls) -- Peek module itself shouldn't notify - end) -end)