From 5046ef4c8f8d738d90b220cf9a2e2a692bd266ba Mon Sep 17 00:00:00 2001 From: luukvbaal Date: Sun, 15 Jun 2025 12:55:01 +0200 Subject: [PATCH] fix(extui): clear cmdline buffer for first message (#34490) Problem: Cmdline buffer is not cleared for a new message (since c973c7ae), resulting in an incorrect spill indicator. When the cmdline buffer is cleared, "msg_row" is not invalidated, resulting in an error. The extui module is untested. Return value of `vim.ui_attach()->callback` is undocumented. Solution: Clear the cmdline buffer for the first message in an event loop iteration. Ensure msg_row passed as end_row does not exceed buffer length. Add `messages_spec2.lua` to test the extui module, keeping in mind that test coverage will greatly increase if this UI is made the default. As such, only tests for specific extui functionality unlikely to be covered by tests leveraging the current message grid. Document the return value of `vim.ui_attach()->callback`, it seems to make sense, and is also used to suppress remote UI events in `messages_spec2.lua`. --- runtime/doc/lua.txt | 6 ++- runtime/lua/vim/_extui.lua | 10 ++-- runtime/lua/vim/_extui/cmdline.lua | 10 ++-- runtime/lua/vim/_extui/messages.lua | 18 ++++--- runtime/lua/vim/_meta/builtin.lua | 4 +- src/nvim/ui.c | 3 -- test/functional/messages2_spec.lua | 73 +++++++++++++++++++++++++++++ 7 files changed, 104 insertions(+), 20 deletions(-) create mode 100644 test/functional/messages2_spec.lua diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt index 6c08ab955a..f7b4d80d17 100644 --- a/runtime/doc/lua.txt +++ b/runtime/doc/lua.txt @@ -1076,8 +1076,10 @@ vim.ui_attach({ns}, {opts}, {callback}) *vim.ui_attach()* enable events for the respective UI element. • {set_cmdheight}? (`boolean`) If false, avoid setting 'cmdheight' to 0 when `ext_messages` is enabled. - • {callback} (`fun(event: string, ...)`) Function called for each UI - event + • {callback} (`fun(event: string, ...): any`) Function called for each + UI event. A truthy return value signals to Nvim that the + event is handled, in which case it is not propagated to + remote UIs. vim.ui_detach({ns}) *vim.ui_detach()* Detach a callback previously attached with |vim.ui_attach()| for the given diff --git a/runtime/lua/vim/_extui.lua b/runtime/lua/vim/_extui.lua index d27d665c15..1b9d81b091 100644 --- a/runtime/lua/vim/_extui.lua +++ b/runtime/lua/vim/_extui.lua @@ -40,9 +40,6 @@ local M = {} local function ui_callback(event, ...) local handler = ext.msg[event] or ext.cmd[event] - if not handler then - return - end ext.tab_check_wins() handler(...) api.nvim__redraw({ @@ -59,6 +56,9 @@ function M.enable(opts) if opts.msg then vim.validate('opts.msg.pos', opts.msg.pos, 'nil', true, 'nil: "pos" moved to opts.target') vim.validate('opts.msg.box', opts.msg.box, 'nil', true, 'nil: "timeout" moved to opts.msg') + vim.validate('opts.msg.target', opts.msg.target, function(tar) + return tar == 'cmd' or tar == 'msg' + end, "'cmd'|'msg'") end ext.cfg = vim.tbl_deep_extend('keep', opts, ext.cfg) @@ -76,11 +76,15 @@ function M.enable(opts) end vim.ui_attach(ext.ns, { ext_messages = true, set_cmdheight = false }, function(event, ...) + if not (ext.msg[event] or ext.cmd[event]) then + return + end if vim.in_fast_event() then scheduled_ui_callback(event, ...) else ui_callback(event, ...) end + return true end) -- Use MsgArea and hide search highlighting in the cmdline window. diff --git a/runtime/lua/vim/_extui/cmdline.lua b/runtime/lua/vim/_extui/cmdline.lua index 4b9198d11d..53f14ccb4b 100644 --- a/runtime/lua/vim/_extui/cmdline.lua +++ b/runtime/lua/vim/_extui/cmdline.lua @@ -110,11 +110,11 @@ end --- Leaving the cmdline, restore 'cmdheight' and 'ruler'. --- ---@param level integer +---@param level integer ---@param abort boolean -function M.cmdline_hide(_, abort) - if M.row > 0 then - return -- No need to hide when still in cmdline_block. +function M.cmdline_hide(level, abort) + if M.row > 0 or level > 1 then + return -- No need to hide when still in nested cmdline or cmdline_block. end fn.clearmatches(ext.wins.cmd) -- Clear matchparen highlights. @@ -166,7 +166,7 @@ end --- Clear cmdline buffer and leave the cmdline. function M.cmdline_block_hide() M.row = 0 - M.cmdline_hide(nil, true) + M.cmdline_hide(M.level, true) end return M diff --git a/runtime/lua/vim/_extui/messages.lua b/runtime/lua/vim/_extui/messages.lua index 1812b778dd..46a2b97db2 100644 --- a/runtime/lua/vim/_extui/messages.lua +++ b/runtime/lua/vim/_extui/messages.lua @@ -90,10 +90,12 @@ local function set_virttext(type) elseif #chunks > 0 then local tar = type == 'msg' and ext.cfg.msg.target or 'cmd' local win = ext.wins[tar] - local max = api.nvim_win_get_height(win) - local erow = tar == 'cmd' and M.cmd.msg_row or nil - local srow = tar == 'msg' and fn.line('w0', ext.wins.msg) - 1 or nil - local h = api.nvim_win_text_height(win, { start_row = srow, end_row = erow, max_height = max }) + local erow = tar == 'cmd' and math.min(M.cmd.msg_row, api.nvim_buf_line_count(ext.bufs.cmd) - 1) + local h = api.nvim_win_text_height(win, { + max_height = api.nvim_win_get_height(win), + start_row = tar == 'msg' and fn.line('w0', ext.wins.msg) - 1 or nil, + end_row = erow or nil, + }) local row = h.end_row ---@type integer local col = fn.virtcol2col(win, row + 1, h.end_vcol) local scol = fn.screenpos(win, row + 1, col).col ---@type integer @@ -223,6 +225,11 @@ function M.show_msg(tar, content, replace_last, append, pager) cr = M[tar].count > 0 and msg:sub(1, 1) == '\r' restart = M[tar].count > 0 and (replace_last or dupe > 0) count = M[tar].count + ((restart or msg == '\n') and 0 or 1) + + -- Ensure cmdline is clear when writing the first message. + if tar == 'cmd' and not will_pager and dupe == 0 and M.cmd.count == 0 then + api.nvim_buf_set_lines(ext.bufs.cmd, 0, -1, false, {}) + end end -- Filter out empty newline messages. TODO: don't emit them. @@ -367,9 +374,8 @@ function M.msg_show(kind, content, _, _, append) -- Store the time when an error message was emitted in order to not overwrite -- it with 'last' virt_text in the cmdline to give the user a chance to read it. M.cmd.last_emsg = kind == 'emsg' and os.time() or M.cmd.last_emsg - -- Should clear the search count now, which also affects the showcmd position. + -- Should clear the search count now, mark itself is cleared by invalidate. M.virt.last[M.virt.idx.search][1] = nil - M.msg_showcmd({}) end -- Typed "inspection" messages should be routed to the pager. diff --git a/runtime/lua/vim/_meta/builtin.lua b/runtime/lua/vim/_meta/builtin.lua index ef7ec0bee3..b676522566 100644 --- a/runtime/lua/vim/_meta/builtin.lua +++ b/runtime/lua/vim/_meta/builtin.lua @@ -266,7 +266,9 @@ function vim.wait(time, callback, interval, fast_only) end --- enable events for the respective UI element. --- - {set_cmdheight}? (`boolean`) If false, avoid setting --- 'cmdheight' to 0 when `ext_messages` is enabled. ---- @param callback fun(event: string, ...) Function called for each UI event +--- @param callback fun(event: string, ...): any Function called for each UI event. +--- A truthy return value signals to Nvim that the event is handled, +--- in which case it is not propagated to remote UIs. function vim.ui_attach(ns, opts, callback) end --- Detach a callback previously attached with |vim.ui_attach()| for the diff --git a/src/nvim/ui.c b/src/nvim/ui.c index bb7f1e91c8..5e10e0ba99 100644 --- a/src/nvim/ui.c +++ b/src/nvim/ui.c @@ -759,9 +759,6 @@ void ui_call_event(char *name, bool fast, Array args) uint32_t ns_id = ui_event_ns_id; Object res = nlua_call_ref_ctx(fast, event_cb->cb, name, args, kRetNilBool, NULL, &err); ui_event_ns_id = 0; - // TODO(bfredl/luukvbaal): should this be documented or reconsidered? - // Why does truthy return from Lua callback mean remote UI should not receive - // the event. if (LUARET_TRUTHY(res)) { handled = true; } diff --git a/test/functional/messages2_spec.lua b/test/functional/messages2_spec.lua new file mode 100644 index 0000000000..faa6ae8d53 --- /dev/null +++ b/test/functional/messages2_spec.lua @@ -0,0 +1,73 @@ +local n = require('test.functional.testnvim')() +local Screen = require('test.functional.ui.screen') + +local clear, command, exec_lua, feed = n.clear, n.command, n.exec_lua, n.feed + +describe('messages2', function() + local screen + describe('target=msg', function() + before_each(function() + clear() + screen = Screen.new() + screen:add_extra_attr_ids({ + [100] = { foreground = Screen.colors.Magenta1, bold = true }, + }) + exec_lua(function() + require('vim._extui').enable({}) + end) + end) + + it('multiline messages and pager', function() + command('echo "foo\nbar"') + screen:expect([[ + ^ | + {1:~ }|*12 + foo[+1] | + ]]) + command('set ruler') + feed('g') + screen:expect([[ + | + {1:~ }|*9 + ─{100:Pager}───────────────────────────────────────────────| + {4:fo^o }| + {4:bar }| + foo[+1] 1,3 All| + ]]) + -- New message clears spill indicator. + feed('Q') + screen:expect([[ + | + {1:~ }|*9 + ─{100:Pager}───────────────────────────────────────────────| + {4:fo^o }| + {4:bar }| + {9:E354: Invalid register name: '^@'} 1,3 All| + ]]) + -- Multiple messages in same event loop iteration are appended. + feed([[q:echo "foo\nbar" | echo "baz"]]) + screen:expect([[ + | + {1:~ }|*8 + ─{100:Pager}───────────────────────────────────────────────| + {4:^foo }| + {4:bar }| + {4:baz }| + 1,1 All| + ]]) + -- No error for ruler virt_text msg_row exceeding buffer length. + command([[map Q echo "foo\nbar" ls]]) + feed('qQ') + screen:expect([[ + | + {1:~ }|*7 + ─{100:Pager}───────────────────────────────────────────────| + {4:^foo }| + {4:bar }| + {4: }| + {4: 1 %a "[No Name]" line 1 }| + 1,1 All| + ]]) + end) + end) +end)