fix(api/nvim_win_call): share common win_execute logic

seandewar · seandewar · commit 452b46fcf79d · 2022-02-03T15:03:08.000Z
We have to be sure that the bugs fixed in the previous patches also apply to
nvim_win_call.

Checking v8.1.2124 and v8.2.4026 is especially important as these patches were
only applied to win_execute, but nvim_win_call is also affected by the same
bugs. A lot of win_execute's logic can be shared with nvim_win_call, so factor
it out into a common macro to reduce the possibility of this happening again.
diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c
@@ -457,12 +457,10 @@ Object nvim_win_call(Window window, LuaRef fun, Error *err)
 
   try_start();
   Object res = OBJECT_INIT;
-  switchwin_T switchwin;
-  if (switch_win_noblock(&switchwin, win, tabpage, true) == OK) {
+  WIN_EXECUTE(win, tabpage, {
     Array args = ARRAY_DICT_INIT;
     res = nlua_call_ref(fun, NULL, args, true, err);
-  }
-  restore_win_noblock(&switchwin, true);
+  });
   try_end(err);
   return res;
 }
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
@@ -2181,25 +2181,7 @@ static void f_win_execute(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   rettv->vval.v_string = NULL;
 
   if (wp != NULL && tp != NULL) {
-    pos_T curpos = wp->w_cursor;
-    switchwin_T switchwin;
-    if (switch_win_noblock(&switchwin, wp, tp, true) == OK) {
-      check_cursor();
-      execute_common(argvars, rettv, fptr, 1);
-    }
-    restore_win_noblock(&switchwin, true);
-
-    // Update the status line if the cursor moved.
-    if (win_valid(wp) && !equalpos(curpos, wp->w_cursor)) {
-      wp->w_redr_status = true;
-    }
-
-    // In case the command moved the cursor or changed the Visual area,
-    // check it is valid.
-    check_cursor();
-    if (VIsual_active) {
-      check_pos(curbuf, &VIsual);
-    }
+    WIN_EXECUTE(wp, tp, execute_common(argvars, rettv, fptr, 1));
   }
 }
 
diff --git a/src/nvim/window.h b/src/nvim/window.h
@@ -4,6 +4,7 @@
 #include <stdbool.h>
 
 #include "nvim/buffer_defs.h"
+#include "nvim/mark.h"
 
 // Values for file_name_in_line()
 #define FNAME_MESS      1       // give error message
@@ -40,6 +41,30 @@ typedef struct {
   bool sw_visual_active;
 } switchwin_T;
 
+/// Execute a block of code in the context of window `wp` in tabpage `tp`.
+/// Ensures the status line is redrawn and cursor position is valid if it is moved.
+#define WIN_EXECUTE(wp, tp, block) \
+  do { \
+    win_T *const wp_ = (wp); \
+    const pos_T curpos_ = wp_->w_cursor; \
+    switchwin_T switchwin_; \
+    if (switch_win_noblock(&switchwin_, wp_, (tp), true) == OK) { \
+      check_cursor(); \
+      block; \
+    } \
+    restore_win_noblock(&switchwin_, true); \
+    /* Update the status line if the cursor moved. */ \
+    if (win_valid(wp_) && !equalpos(curpos_, wp_->w_cursor)) { \
+      wp_->w_redr_status = true; \
+    } \
+    /* In case the command moved the cursor or changed the Visual area, */ \
+    /* check it is valid. */ \
+    check_cursor(); \
+    if (VIsual_active) { \
+      check_pos(curbuf, &VIsual); \
+    } \
+  } while (false)
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "window.h.generated.h"
 #endif
diff --git a/test/functional/lua/vim_spec.lua b/test/functional/lua/vim_spec.lua
@@ -2445,6 +2445,75 @@ describe('lua stdlib', function()
       eq(win1, meths.get_current_win())
       eq(win2, val)
     end)
+
+    it('does not cause ml_get errors with invalid visual selection', function()
+      -- Add lines to the current buffer and make another window looking into an empty buffer.
+      exec_lua [[
+        _G.a = vim.api
+        _G.t = function(s) return a.nvim_replace_termcodes(s, true, true, true) end
+        _G.win_lines = a.nvim_get_current_win()
+        vim.cmd "new"
+        _G.win_empty = a.nvim_get_current_win()
+        a.nvim_set_current_win(win_lines)
+        a.nvim_buf_set_lines(0, 0, -1, true, {"a", "b", "c"})
+      ]]
+
+      -- Start Visual in current window, redraw in other window with fewer lines.
+      -- Should be fixed by vim-patch:8.2.4018.
+      exec_lua [[
+        a.nvim_feedkeys(t "G<C-V>", "txn", false)
+        a.nvim_win_call(win_empty, function() vim.cmd "redraw" end)
+      ]]
+
+      -- Start Visual in current window, extend it in other window with more lines.
+      -- Fixed for win_execute by vim-patch:8.2.4026, but nvim_win_call should also not be affected.
+      exec_lua [[
+        a.nvim_feedkeys(t "<Esc>gg", "txn", false)
+        a.nvim_set_current_win(win_empty)
+        a.nvim_feedkeys(t "gg<C-V>", "txn", false)
+        a.nvim_win_call(win_lines, function() a.nvim_feedkeys(t "G<C-V>", "txn", false) end)
+        vim.cmd "redraw"
+      ]]
+    end)
+
+    it('updates ruler if cursor moved', function()
+      -- Fixed for win_execute in vim-patch:8.1.2124, but should've applied to nvim_win_call too!
+      local screen = Screen.new(30, 5)
+      screen:set_default_attr_ids {
+          [1] = {reverse = true},
+          [2] = {bold = true, reverse = true},
+      }
+      screen:attach()
+      exec_lua [[
+        _G.a = vim.api
+        vim.opt.ruler = true
+        local lines = {}
+        for i = 0, 499 do lines[#lines + 1] = tostring(i) end
+        a.nvim_buf_set_lines(0, 0, -1, true, lines)
+        a.nvim_win_set_cursor(0, {20, 0})
+        vim.cmd "split"
+        _G.win = a.nvim_get_current_win()
+        vim.cmd "wincmd w | redraw"
+      ]]
+      screen:expect [[
+        19                            |
+        {1:[No Name] [+]  20,1         3%}|
+        ^19                            |
+        {2:[No Name] [+]  20,1         3%}|
+                                      |
+      ]]
+      exec_lua [[
+        a.nvim_win_call(win, function() a.nvim_win_set_cursor(0, {100, 0}) end)
+        vim.cmd "redraw"
+      ]]
+      screen:expect [[
+        99                            |
+        {1:[No Name] [+]  100,1       19%}|
+        ^19                            |
+        {2:[No Name] [+]  20,1         3%}|
+                                      |
+      ]]
+    end)
   end)
 end)
 
