vim-patch:9.0.0914: deletebufline() may move marks in the wrong window

Problem:    deletebufline() may move marks in the wrong window.
Solution:   Find a window for the buffer being changed. (closes vim/vim#11583)

228e422855

Cherry-pick code change from patch 9.0.0961.

Co-authored-by: Bram Moolenaar <Bram@vim.org>

src/nvim/eval/buffer.c

@@ -28,6 +28,13 @@
 #include "nvim/undo.h"
 #include "nvim/vim.h"
 
+typedef struct {
+  win_T *cob_curwin_save;
+  aco_save_T cob_aco;
+  int cob_using_aco;
+  int cob_save_VIsual_active;
+} cob_T;
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "eval/buffer.c.generated.h"
 #endif
@@ -60,14 +67,57 @@ buf_T *find_buffer(typval_T *avar)
 /// If there is a window for "curbuf", make it the current window.
 static void find_win_for_curbuf(void)
 {
-  for (wininfo_T *wip = curbuf->b_wininfo; wip != NULL; wip = wip->wi_next) {
+  wininfo_T *wip;
-    if (wip->wi_win != NULL) {
+
+  // The b_wininfo list should have the windows that recently contained the
+  // buffer, going over this is faster than going over all the windows.
+  // Do check the buffer is still there.
+  FOR_ALL_BUF_WININFO(curbuf, wip) {
+    if (wip->wi_win != NULL && wip->wi_win->w_buffer == curbuf) {
       curwin = wip->wi_win;
       break;
     }
   }
 }
 
+/// Used before making a change in "buf", which is not the current one: Make
+/// "buf" the current buffer and find a window for this buffer, so that side
+/// effects are done correctly (e.g., adjusting marks).
+///
+/// Information is saved in "cob" and MUST be restored by calling
+/// change_other_buffer_restore().
+static void change_other_buffer_prepare(cob_T *cob, buf_T *buf)
+{
+  CLEAR_POINTER(cob);
+
+  // Set "curbuf" to the buffer being changed.  Then make sure there is a
+  // window for it to handle any side effects.
+  cob->cob_save_VIsual_active = VIsual_active;
+  VIsual_active = false;
+  cob->cob_curwin_save = curwin;
+  curbuf = buf;
+  find_win_for_curbuf();  // simplest: find existing window for "buf"
+
+  if (curwin->w_buffer != buf) {
+    // No existing window for this buffer.  It is dangerous to have
+    // curwin->w_buffer differ from "curbuf", use the autocmd window.
+    curbuf = curwin->w_buffer;
+    aucmd_prepbuf(&cob->cob_aco, buf);
+    cob->cob_using_aco = true;
+  }
+}
+
+static void change_other_buffer_restore(cob_T *cob)
+{
+  if (cob->cob_using_aco) {
+    aucmd_restbuf(&cob->cob_aco);
+  } else {
+    curwin = cob->cob_curwin_save;
+    curbuf = curwin->w_buffer;
+  }
+  VIsual_active = cob->cob_save_VIsual_active;
+}
+
 /// Set line or list of lines in buffer "buf" to "lines".
 /// Any type is allowed and converted to a string.
 static void set_buffer_lines(buf_T *buf, linenr_T lnum_arg, bool append, typval_T *lines,
@@ -76,10 +126,6 @@ static void set_buffer_lines(buf_T *buf, linenr_T lnum_arg, bool append, typval_
 {
   linenr_T lnum = lnum_arg + (append ? 1 : 0);
   long added = 0;
-  win_T *curwin_save = NULL;
-  aco_save_T aco;
-  int using_aco = false;
-  const bool save_VIsual_active = VIsual_active;
 
   // When using the current buffer ml_mfp will be set if needed.  Useful when
   // setline() is used on startup.  For other buffers the buffer must be
@@ -90,21 +136,11 @@ static void set_buffer_lines(buf_T *buf, linenr_T lnum_arg, bool append, typval_
     return;
   }
 
+  // After this don't use "return", goto "cleanup"!
+  cob_T cob;
   if (!is_curbuf) {
-    // Set "curbuf" to the buffer being changed.  Then make sure there is a
+    // set "curbuf" to "buf" and find a window for this buffer
-    // window for it to handle any side effects.
+    change_other_buffer_prepare(&cob, buf);
-    VIsual_active = false;
-    curwin_save = curwin;
-    curbuf = buf;
-    find_win_for_curbuf();  // simplest: find existing window for "buf"
-
-    if (curwin->w_buffer != buf) {
-      // No existing window for this buffer.  It is dangerous to have
-      // curwin->w_buffer differ from "curbuf", use the autocmd window.
-      curbuf = curwin->w_buffer;
-      aucmd_prepbuf(&aco, buf);
-      using_aco = true;
-    }
   }
 
   linenr_T append_lnum;
@@ -127,7 +163,7 @@ static void set_buffer_lines(buf_T *buf, linenr_T lnum_arg, bool append, typval_
       if (lnum > curbuf->b_ml.ml_line_count) {
         rettv->vval.v_number = 1;       // FAIL
       }
-      goto done;
+      goto cleanup;
     }
     li = tv_list_first(l);
   } else {
@@ -201,15 +237,9 @@ static void set_buffer_lines(buf_T *buf, linenr_T lnum_arg, bool append, typval_
     update_topline(curwin);
   }
 
-done:
+cleanup:
   if (!is_curbuf) {
-    if (using_aco) {
+    change_other_buffer_restore(&cob);
-      aucmd_restbuf(&aco);
-    } else {
-      curwin = curwin_save;
-      curbuf = curwin->w_buffer;
-    }
-    VIsual_active = save_VIsual_active;
   }
 }
 
@@ -388,8 +418,6 @@ void f_deletebufline(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   if (buf == NULL) {
     return;
   }
-  const bool is_curbuf = buf == curbuf;
-  const bool save_VIsual_active = VIsual_active;
 
   linenr_T last;
   const linenr_T first = tv_get_lnum_buf(&argvars[1], buf);
@@ -407,16 +435,14 @@ void f_deletebufline(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     return;
   }
 
-  buf_T *curbuf_save = NULL;
-  win_T *curwin_save = NULL;
   // After this don't use "return", goto "cleanup"!
+  const bool is_curbuf = buf == curbuf;
+  cob_T cob;
   if (!is_curbuf) {
-    VIsual_active = false;
+    // set "curbuf" to "buf" and find a window for this buffer
-    curbuf_save = curbuf;
+    change_other_buffer_prepare(&cob, buf);
-    curwin_save = curwin;
-    curbuf = buf;
-    find_win_for_curbuf();
   }
+
   if (last > curbuf->b_ml.ml_line_count) {
     last = curbuf->b_ml.ml_line_count;
   }
@@ -455,9 +481,7 @@ void f_deletebufline(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 
 cleanup:
   if (!is_curbuf) {
-    curbuf = curbuf_save;
+    change_other_buffer_restore(&cob);
-    curwin = curwin_save;
-    VIsual_active = save_VIsual_active;
   }
 }
 

src/nvim/testdir/test_bufline.vim

@@ -98,12 +98,25 @@ func Test_appendbufline()
   new
   let b = bufnr('%')
   hide
+
+  new
+  call setline(1, ['line1', 'line2', 'line3'])
+  normal! 2gggg
+  call assert_equal(2, line("''"))
+
   call assert_equal(0, appendbufline(b, 0, ['foo', 'bar']))
   call assert_equal(['foo'], getbufline(b, 1))
   call assert_equal(['bar'], getbufline(b, 2))
   call assert_equal(['foo', 'bar'], getbufline(b, 1, 2))
+  call assert_equal(0, appendbufline(b, 0, 'baz'))
+  call assert_equal(['baz', 'foo', 'bar'], getbufline(b, 1, 3))
+
+  " appendbufline() in a hidden buffer shouldn't move marks in current window.
+  call assert_equal(2, line("''"))
+  bwipe!
+
   exe "bd!" b
-  call assert_equal([], getbufline(b, 1, 2))
+  call assert_equal([], getbufline(b, 1, 3))
 
   split Xtest
   call setline(1, ['a', 'b', 'c'])
@@ -145,10 +158,21 @@ func Test_deletebufline()
   let b = bufnr('%')
   call setline(1, ['aaa', 'bbb', 'ccc'])
   hide
+
+  new
+  call setline(1, ['line1', 'line2', 'line3'])
+  normal! 2gggg
+  call assert_equal(2, line("''"))
+
   call assert_equal(0, deletebufline(b, 2))
   call assert_equal(['aaa', 'ccc'], getbufline(b, 1, 2))
   call assert_equal(0, deletebufline(b, 2, 8))
   call assert_equal(['aaa'], getbufline(b, 1, 2))
+
+  " deletebufline() in a hidden buffer shouldn't move marks in current window.
+  call assert_equal(2, line("''"))
+  bwipe!
+
   exe "bd!" b
   call assert_equal(1, b->deletebufline(1))