fix(api): make open_win block only enter/leave events if !enter && !noautocmd

Problem: nvim_open_win blocking all win_set_buf autocommands when !enter &&
!noautocmd is too aggressive.

Solution: temporarily block WinEnter/Leave and BufEnter/Leave events when
setting the buffer. Delegate the firing of BufWinEnter back to win_set_buf,
which also has the advantage of keeping the timing consistent (e.g: before the
epilogue in enter_buffer, which also handles restoring the cursor position if
autocommands didn't change it, among other things). Reword the documentation for
noautocmd a bit.

I pondered modifying do_buffer and callees to allow for BufEnter/Leave being
conditionally disabled, but it seems too invasive (and potentially error-prone,
especially if new code paths to BufEnter/Leave are added in the future).

Unfortunately, doing this has the drawback of blocking ALL such events for the
duration, which also means blocking unrelated such events; like if window
switching occurs in a ++nested autocmd fired by win_set_buf. If this turns out
to be a problem in practice, a different solution specialized for nvim_open_win
could be considered. :-)
This commit is contained in:
Sean Dewar 2024-02-11 22:53:37 +00:00 committed by Sean Dewar
parent b1577d371a
commit a70eae57bd
No known key found for this signature in database
GPG Key ID: 08CC2C83AD41B581
4 changed files with 55 additions and 21 deletions

View File

@ -3265,9 +3265,9 @@ nvim_open_win({buffer}, {enter}, {config}) *nvim_open_win()*
• footer_pos: Footer position. Must be set with `footer`
option. Value can be one of "left", "center", or "right".
Default is `"left"`.
• noautocmd: If true then no buffer-related autocommand
events such as |BufEnter|, |BufLeave| or |BufWinEnter| may
fire from calling this function.
• noautocmd: If true then autocommands triggered from
setting the `buffer` to display are blocked (e.g:
|BufEnter|, |BufLeave|, |BufWinEnter|).
• fixed: If true when anchor is NW or SW, the float window
would be kept fixed even if the window would be truncated.
• hide: If true the floating window will be hidden.

View File

@ -1719,9 +1719,9 @@ function vim.api.nvim_open_term(buffer, opts) end
--- • footer_pos: Footer position. Must be set with `footer`
--- option. Value can be one of "left", "center", or "right".
--- Default is `"left"`.
--- • noautocmd: If true then no buffer-related autocommand
--- events such as `BufEnter`, `BufLeave` or `BufWinEnter` may
--- fire from calling this function.
--- • noautocmd: If true then autocommands triggered from
--- setting the `buffer` to display are blocked (e.g:
--- `BufEnter`, `BufLeave`, `BufWinEnter`).
--- • fixed: If true when anchor is NW or SW, the float window
--- would be kept fixed even if the window would be truncated.
--- • hide: If true the floating window will be hidden.

View File

@ -199,9 +199,9 @@
/// - footer_pos: Footer position. Must be set with `footer` option.
/// Value can be one of "left", "center", or "right".
/// Default is `"left"`.
/// - noautocmd: If true then no buffer-related autocommand events such as
/// |BufEnter|, |BufLeave| or |BufWinEnter| may fire from
/// calling this function.
/// - noautocmd: If true then autocommands triggered from setting the
/// `buffer` to display are blocked (e.g: |BufEnter|, |BufLeave|,
/// |BufWinEnter|).
/// - fixed: If true when anchor is NW or SW, the float window
/// would be kept fixed even if the window would be truncated.
/// - hide: If true the floating window will be hidden.
@ -302,20 +302,20 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err
tp = win_find_tabpage(wp);
}
if (tp && bufref_valid(&bufref) && buf != wp->w_buffer) {
const bool noautocmd = curwin != wp || fconfig.noautocmd;
win_set_buf(wp, buf, noautocmd, err);
if (!noautocmd) {
// win_set_buf temporarily makes `wp` the curwin to set the buffer.
// If not entering `wp`, block Enter and Leave events. (cringe)
const bool au_no_enter_leave = curwin != wp && !fconfig.noautocmd;
if (au_no_enter_leave) {
autocmd_no_enter++;
autocmd_no_leave++;
}
win_set_buf(wp, buf, fconfig.noautocmd, err);
if (!fconfig.noautocmd) {
tp = win_find_tabpage(wp);
}
// win_set_buf autocommands were blocked if we didn't enter, but we still want BufWinEnter.
if (noautocmd && !fconfig.noautocmd && wp->w_buffer == buf) {
const int result = switch_win_noblock(&switchwin, wp, tp, true);
assert(result == OK);
(void)result;
if (apply_autocmds(EVENT_BUFWINENTER, NULL, NULL, false, buf)) {
tp = win_find_tabpage(wp);
}
restore_win_noblock(&switchwin, true);
if (au_no_enter_leave) {
autocmd_no_enter--;
autocmd_no_leave--;
}
}
if (!tp) {

View File

@ -1620,6 +1620,40 @@ describe('API/win', function()
)
command('new | quit')
end)
it('restores last known cursor position if BufWinEnter did not move it', function()
-- This test mostly exists to ensure BufWinEnter is executed before enter_buffer's epilogue.
local buf = api.nvim_get_current_buf()
insert([[
foo
bar baz .etc
i love autocommand bugs!
supercalifragilisticexpialidocious
marvim is actually a human
llanfairpwllgwyngyllgogerychwyrndrobwllllantysiliogogogoch
]])
api.nvim_win_set_cursor(0, { 5, 2 })
command('set nostartofline | enew')
local new_win = api.nvim_open_win(buf, false, { split = 'left' })
eq({ 5, 2 }, api.nvim_win_get_cursor(new_win))
exec([[
only!
autocmd BufWinEnter * ++once normal! j6l
]])
new_win = api.nvim_open_win(buf, false, { split = 'left' })
eq({ 2, 6 }, api.nvim_win_get_cursor(new_win))
end)
it('does not block all win_set_buf autocommands if !enter and !noautocmd', function()
local new_buf = fn.bufadd('foobarbaz')
exec([[
let triggered = ""
autocmd BufReadCmd * ++once let triggered = bufname()
]])
api.nvim_open_win(new_buf, false, { split = 'left' })
eq('foobarbaz', eval('triggered'))
end)
end)
describe('set_config', function()