mirrors/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2025-10-26 12:27:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): not using TRY_WRAP, generic error messages #31595

Browse Source 
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`.
This commit is contained in:
Justin M. Keyes
2024-12-16 04:00:20 -08:00
committed by GitHub
parent 9c6a3703bb
commit 167a2383b9
7 changed files with 65 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/nvim/api/private/helpers.c
View File

@@ -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

19
src/nvim/api/vim.c
View File

@@ -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) {
if (!buf) {
return;
}
if (curwin->w_p_wfb) {
api_set_error(err, kErrorTypeException, "%s", e_winfixbuf_cannot_go_to_buffer);
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.

7
src/nvim/api/window.c
View File

@@ -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;
}

40
src/nvim/window.c
View File

@@ -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();
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;
}
const int save_acd = p_acd;
if (!switchwin.sw_same_win) {
// Temporarily disable 'autochdir' when setting buffer in another window.
p_acd = false;
}
do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buf->b_fnum, 0);
int result = 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--;
}

2
test/functional/api/window_spec.lua
View File

@@ -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'))

94
test/functional/options/winfixbuf_spec.lua
View File

@@ -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
]])
assert(not ok)
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)
end)
it('vim.api.nvim_win_set_buf on current buffer', function()
exec_lua([[
---@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
local curbuf = vim.api.nvim_get_current_buf()
vim.api.nvim_win_set_buf(0, curbuf)
assert(vim.api.nvim_get_current_buf() == curbuf)
return buffer
]])
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 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 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('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('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)

6
test/old/testdir/test_winfixbuf.vim
View File

@@ -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