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`.

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

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) {
+  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.

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

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();
+  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--;
 }

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

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
-    ]])
-
-    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)

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