diff options
author | Justin M. Keyes <justinkz@gmail.com> | 2024-12-16 04:00:20 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-16 04:00:20 -0800 |
commit | 167a2383b9966ac227a77b0221088246e14ce75a (patch) | |
tree | d509817042d19ba96a3305abdc62f1f7a67054ab | |
parent | 9c6a3703bb15d56fecdd962512f69f0ccf6d398c (diff) | |
download | rneovim-167a2383b9966ac227a77b0221088246e14ce75a.tar.gz rneovim-167a2383b9966ac227a77b0221088246e14ce75a.tar.bz2 rneovim-167a2383b9966ac227a77b0221088246e14ce75a.zip |
fix(api): not using TRY_WRAP, generic error messages #31595
Problem:
- API functions using `try_start` directly instead of `TRY_WRAP`, do not
surface the underlying error message, and instead show generic things
like "Failed to set buffer".
- Error handling code is duplicated in the API impl, instead of
delegating to the vim buffer/window handling logic.
Solution:
- Use `TRY_WRAP`.
-rw-r--r-- | src/nvim/api/private/helpers.c | 9 | ||||
-rw-r--r-- | src/nvim/api/vim.c | 19 | ||||
-rw-r--r-- | src/nvim/api/window.c | 7 | ||||
-rw-r--r-- | src/nvim/window.c | 40 | ||||
-rw-r--r-- | test/functional/api/window_spec.lua | 2 | ||||
-rw-r--r-- | test/functional/options/winfixbuf_spec.lua | 88 | ||||
-rw-r--r-- | test/old/testdir/test_winfixbuf.vim | 6 |
7 files changed, 62 insertions, 109 deletions
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index 8ddaecc58e..5bf66a092f 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -101,9 +101,9 @@ bool try_leave(const TryState *const tstate, Error *const err) return ret; } -/// Start block that may cause vimscript exceptions +/// Starts a block that may cause Vimscript exceptions; must be mirrored by `try_end()` call. /// -/// Each try_start() call should be mirrored by try_end() call. +/// Note: use `TRY_WRAP` instead (except in `FUNC_API_FAST` functions such as nvim_get_runtime_file). /// /// To be used as a replacement of `:try … catch … endtry` in C code, in cases /// when error flag could not already be set. If there may be pending error @@ -114,8 +114,9 @@ void try_start(void) trylevel++; } -/// End try block, set the error message if any and return true if an error -/// occurred. +/// Ends a `try_start` block; sets error message if any and returns true if an error occurred. +/// +/// Note: use `TRY_WRAP` instead (except in `FUNC_API_FAST` functions such as nvim_get_runtime_file). /// /// @param err Pointer to the stack-allocated error object /// @return true if an error occurred diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 42fc21deac..1acfa0d34b 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -596,6 +596,7 @@ ArrayOf(String) nvim_get_runtime_file(String name, Boolean all, Arena *arena, Er int flags = DIP_DIRFILE | (all ? DIP_ALL : 0); TryState tstate; + // XXX: intentionally not using `TRY_WRAP`, to avoid `did_emsg=false` in `try_end`. try_enter(&tstate); do_in_runtimepath((name.size ? name.data : ""), flags, find_runtime_cb, &cookie); vim_ignored = try_leave(&tstate, err); @@ -888,23 +889,13 @@ void nvim_set_current_buf(Buffer buffer, Error *err) { buf_T *buf = find_buffer_by_handle(buffer, err); - if (!buf || curwin->w_buffer == buf) { - return; - } - - if (curwin->w_p_wfb) { - api_set_error(err, kErrorTypeException, "%s", e_winfixbuf_cannot_go_to_buffer); + if (!buf) { return; } - try_start(); - int result = do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buf->b_fnum, 0); - if (!try_end(err) && result == FAIL) { - api_set_error(err, - kErrorTypeException, - "Failed to switch to buffer %d", - buffer); - } + TRY_WRAP(err, { + do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buf->b_fnum, 0); + }); } /// Gets the current list of window handles. diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index f415415fa7..170d5c6cdb 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -59,12 +59,7 @@ void nvim_win_set_buf(Window window, Buffer buffer, Error *err) { win_T *win = find_window_by_handle(window, err); buf_T *buf = find_buffer_by_handle(buffer, err); - if (!win || !buf || win->w_buffer == buf) { - return; - } - - if (win->w_p_wfb) { - api_set_error(err, kErrorTypeException, "%s", e_winfixbuf_cannot_go_to_buffer); + if (!win || !buf) { return; } diff --git a/src/nvim/window.c b/src/nvim/window.c index 938d9d7618..1430b2efb2 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -746,40 +746,28 @@ void win_set_buf(win_T *win, buf_T *buf, Error *err) RedrawingDisabled++; switchwin_T switchwin; - if (switch_win_noblock(&switchwin, win, tab, true) == FAIL) { - api_set_error(err, - kErrorTypeException, - "Failed to switch to window %d", - win->handle); - goto cleanup; - } - - try_start(); - const int save_acd = p_acd; - if (!switchwin.sw_same_win) { - // Temporarily disable 'autochdir' when setting buffer in another window. - p_acd = false; - } + TRY_WRAP(err, { + int win_result = switch_win_noblock(&switchwin, win, tab, true); + if (win_result != FAIL) { + const int save_acd = p_acd; + if (!switchwin.sw_same_win) { + // Temporarily disable 'autochdir' when setting buffer in another window. + p_acd = false; + } - int result = do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buf->b_fnum, 0); + do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buf->b_fnum, 0); - if (!switchwin.sw_same_win) { - p_acd = save_acd; - } - - if (!try_end(err) && result == FAIL) { - api_set_error(err, - kErrorTypeException, - "Failed to set buffer %d", - buf->handle); - } + if (!switchwin.sw_same_win) { + p_acd = save_acd; + } + } + }); // If window is not current, state logic will not validate its cursor. So do it now. // Still needed if do_buffer returns FAIL (e.g: autocmds abort script after buffer was set). validate_cursor(curwin); -cleanup: restore_win_noblock(&switchwin, true); RedrawingDisabled--; } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 92999f383a..4662ace4bf 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -1664,7 +1664,7 @@ describe('API/win', function() autocmd BufWinEnter * ++once let fired = v:true ]]) eq( - 'Failed to set buffer 2', + 'Vim:E37: No write since last change (add ! to override)', pcall_err(api.nvim_open_win, api.nvim_create_buf(true, true), false, { split = 'left' }) ) eq(false, eval('fired')) diff --git a/test/functional/options/winfixbuf_spec.lua b/test/functional/options/winfixbuf_spec.lua index 5bed2fc72f..a01650ea71 100644 --- a/test/functional/options/winfixbuf_spec.lua +++ b/test/functional/options/winfixbuf_spec.lua @@ -1,73 +1,51 @@ local n = require('test.functional.testnvim')() +local t = require('test.testutil') local clear = n.clear local exec_lua = n.exec_lua -describe("Nvim API calls with 'winfixbuf'", function() +describe("'winfixbuf'", function() before_each(function() clear() end) - it('vim.api.nvim_win_set_buf on non-current buffer', function() - local ok = exec_lua([[ - local function _setup_two_buffers() - local buffer = vim.api.nvim_create_buf(true, true) - - vim.api.nvim_create_buf(true, true) -- Make another buffer - - local current_window = 0 - vim.api.nvim_set_option_value("winfixbuf", true, {win=current_window}) - - return buffer - end - - local other_buffer = _setup_two_buffers() - local current_window = 0 - local ok, _ = pcall(vim.api.nvim_win_set_buf, current_window, other_buffer) - - return ok + ---@return integer + local function setup_winfixbuf() + return exec_lua([[ + local buffer = vim.api.nvim_create_buf(true, true) + vim.api.nvim_create_buf(true, true) -- Make another buffer + vim.wo.winfixbuf = true + return buffer ]]) - - assert(not ok) + end + + it('nvim_win_set_buf on non-current buffer', function() + local other_buf = setup_winfixbuf() + t.eq( + "Vim:E1513: Cannot switch buffer. 'winfixbuf' is enabled", + t.pcall_err(n.api.nvim_win_set_buf, 0, other_buf) + ) end) - it('vim.api.nvim_set_current_buf on non-current buffer', function() - local ok = exec_lua([[ - local function _setup_two_buffers() - local buffer = vim.api.nvim_create_buf(true, true) - - vim.api.nvim_create_buf(true, true) -- Make another buffer - - local current_window = 0 - vim.api.nvim_set_option_value("winfixbuf", true, {win=current_window}) - - return buffer - end - - local other_buffer = _setup_two_buffers() - local ok, _ = pcall(vim.api.nvim_set_current_buf, other_buffer) - - return ok - ]]) - - assert(not ok) + it('nvim_set_current_buf on non-current buffer', function() + local other_buf = setup_winfixbuf() + t.eq( + "Vim:E1513: Cannot switch buffer. 'winfixbuf' is enabled", + t.pcall_err(n.api.nvim_set_current_buf, other_buf) + ) end) - it('vim.api.nvim_win_set_buf on current buffer', function() - exec_lua([[ - vim.wo.winfixbuf = true - local curbuf = vim.api.nvim_get_current_buf() - vim.api.nvim_win_set_buf(0, curbuf) - assert(vim.api.nvim_get_current_buf() == curbuf) - ]]) + it('nvim_win_set_buf on current buffer', function() + setup_winfixbuf() + local curbuf = n.api.nvim_get_current_buf() + n.api.nvim_win_set_buf(0, curbuf) + t.eq(curbuf, n.api.nvim_get_current_buf()) end) - it('vim.api.nvim_set_current_buf on current buffer', function() - exec_lua([[ - vim.wo.winfixbuf = true - local curbuf = vim.api.nvim_get_current_buf() - vim.api.nvim_set_current_buf(curbuf) - assert(vim.api.nvim_get_current_buf() == curbuf) - ]]) + it('nvim_set_current_buf on current buffer', function() + setup_winfixbuf() + local curbuf = n.api.nvim_get_current_buf() + n.api.nvim_set_current_buf(curbuf) + t.eq(curbuf, n.api.nvim_get_current_buf()) end) end) diff --git a/test/old/testdir/test_winfixbuf.vim b/test/old/testdir/test_winfixbuf.vim index 1777bec184..cc8ff86fa1 100644 --- a/test/old/testdir/test_winfixbuf.vim +++ b/test/old/testdir/test_winfixbuf.vim @@ -2613,7 +2613,7 @@ EOF try pyxdo test_winfixbuf_Test_pythonx_pyxdo_set_buffer() - catch /pynvim\.api\.common\.NvimError: E1513:/ + catch /pynvim\.api\.common\.NvimError: Vim:E1513:/ let l:caught = 1 endtry @@ -2644,7 +2644,7 @@ func Test_pythonx_pyxfile() try pyxfile file.py - catch /pynvim\.api\.common\.NvimError: E1513:/ + catch /pynvim\.api\.common\.NvimError: Vim:E1513:/ let l:caught = 1 endtry @@ -2676,7 +2676,7 @@ import vim buffer = vim.vars["_previous_buffer"] vim.current.buffer = vim.buffers[buffer] EOF - catch /pynvim\.api\.common\.NvimError: E1513:/ + catch /pynvim\.api\.common\.NvimError: Vim:E1513:/ let l:caught = 1 endtry |