vim-patch:8.1.2302: :lockmarks does not work for '[ and ']

Problem:    :lockmarks does not work for '[ and '].
Solution:   save and restore '[ and '] marks. (James McCoy, closes vim/vim#5222)
https://github.com/vim/vim/commit/f4a1d1c0542df151bc59ac3b798ed198b5c71ccc

Test_diff_maintains_change_mark doesn't actually fail without these changes.
This is fixed in v8.2.3936.

6 files changed, 197 insertions, 83 deletions

diff --git a/src/nvim/diff.c b/src/nvim/diff.c
index 340fec230c..233753839b 100644
--- a/src/nvim/diff.c
+++ b/src/nvim/diff.c
@@ -790,9 +790,14 @@ static int diff_write(buf_T *buf, diffin_T *din)
   // Always use 'fileformat' set to "unix".
   char_u *save_ff = buf->b_p_ff;
   buf->b_p_ff = vim_strsave((char_u *)FF_UNIX);
+  const bool save_lockmarks = cmdmod.lockmarks;
+  // Writing the buffer is an implementation detail of performing the diff,
+  // so it shouldn't update the '[ and '] marks.
+  cmdmod.lockmarks = true;
   int r = buf_write(buf, din->din_fname, NULL,
                     (linenr_T)1, buf->b_ml.ml_line_count,
                     NULL, false, false, false, true);
+  cmdmod.lockmarks = save_lockmarks;
   free_string_option(buf->b_p_ff);
   buf->b_p_ff = save_ff;
   return r;
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index ca5e14ee63..9f30f98c07 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -980,8 +980,10 @@ int do_move(linenr_T line1, linenr_T line2, linenr_T dest)
         foldMoveRange(win, &win->w_folds, line1, line2, dest);
       }
     }
-    curbuf->b_op_start.lnum = dest - num_lines + 1;
-    curbuf->b_op_end.lnum = dest;
+    if (!cmdmod.lockmarks) {
+      curbuf->b_op_start.lnum = dest - num_lines + 1;
+      curbuf->b_op_end.lnum = dest;
+    }
     line_off = -num_lines;
     byte_off = -extent_byte;
   } else {
@@ -991,10 +993,14 @@ int do_move(linenr_T line1, linenr_T line2, linenr_T dest)
         foldMoveRange(win, &win->w_folds, dest + 1, line1 - 1, line2);
       }
     }
-    curbuf->b_op_start.lnum = dest + 1;
-    curbuf->b_op_end.lnum = dest + num_lines;
+    if (!cmdmod.lockmarks) {
+      curbuf->b_op_start.lnum = dest + 1;
+      curbuf->b_op_end.lnum = dest + num_lines;
+    }
+  }
+  if (!cmdmod.lockmarks) {
+    curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
   }
-  curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
   mark_adjust_nofold(last_line - num_lines + 1, last_line,
                      -(last_line - dest - extra), 0L, kExtmarkNOOP);
 
@@ -1057,9 +1063,11 @@ void ex_copy(linenr_T line1, linenr_T line2, linenr_T n)
   char_u *p;
 
   count = line2 - line1 + 1;
-  curbuf->b_op_start.lnum = n + 1;
-  curbuf->b_op_end.lnum = n + count;
-  curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
+  if (!cmdmod.lockmarks) {
+    curbuf->b_op_start.lnum = n + 1;
+    curbuf->b_op_end.lnum = n + count;
+    curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
+  }
 
   /*
    * there are three situations:
@@ -1272,12 +1280,18 @@ static void do_filter(linenr_T line1, linenr_T line2, exarg_T *eap, char_u *cmd,
   char_u *cmd_buf;
   buf_T *old_curbuf = curbuf;
   int shell_flags = 0;
+  const pos_T orig_start = curbuf->b_op_start;
+  const pos_T orig_end = curbuf->b_op_end;
   const int stmp = p_stmp;
 
   if (*cmd == NUL) {        // no filter command
     return;
   }
 
+  const bool save_lockmarks = cmdmod.lockmarks;
+  // Temporarily disable lockmarks since that's needed to propagate changed
+  // regions of the buffer for foldUpdate(), linecount, etc.
+  cmdmod.lockmarks = false;
 
   cursor_save = curwin->w_cursor;
   linecount = line2 - line1 + 1;
@@ -1458,10 +1472,15 @@ error:
 
 filterend:
 
+  cmdmod.lockmarks = save_lockmarks;
   if (curbuf != old_curbuf) {
     no_wait_return--;
     emsg(_("E135: *Filter* Autocommands must not change current buffer"));
+  } else if (cmdmod.lockmarks) {
+    curbuf->b_op_start = orig_start;
+    curbuf->b_op_end = orig_end;
   }
+
   if (itmp != NULL) {
     os_remove((char *)itmp);
   }
@@ -3034,14 +3053,15 @@ void ex_append(exarg_T *eap)
   // eap->line2 pointed to the end of the buffer and nothing was appended)
   // "end" is set to lnum when something has been appended, otherwise
   // it is the same as "start"  -- Acevedo
-  curbuf->b_op_start.lnum = (eap->line2 < curbuf->b_ml.ml_line_count) ?
-                            eap->line2 + 1 : curbuf->b_ml.ml_line_count;
-  if (eap->cmdidx != CMD_append) {
-    --curbuf->b_op_start.lnum;
+  if (!cmdmod.lockmarks) {
+    curbuf->b_op_start.lnum
+        = (eap->line2 < curbuf->b_ml.ml_line_count) ? eap->line2 + 1 : curbuf->b_ml.ml_line_count;
+    if (eap->cmdidx != CMD_append) {
+      curbuf->b_op_start.lnum--;
+    }
+    curbuf->b_op_end.lnum = (eap->line2 < lnum) ? lnum : curbuf->b_op_start.lnum;
+    curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
   }
-  curbuf->b_op_end.lnum = (eap->line2 < lnum)
-                          ? lnum : curbuf->b_op_start.lnum;
-  curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
   curwin->w_cursor.lnum = lnum;
   check_cursor_lnum();
   beginline(BL_SOL | BL_FIX);
@@ -4354,10 +4374,12 @@ skip:
   }
 
   if (sub_nsubs > start_nsubs) {
-    // Set the '[ and '] marks.
-    curbuf->b_op_start.lnum = eap->line1;
-    curbuf->b_op_end.lnum = line2;
-    curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
+    if (!cmdmod.lockmarks) {
+      // Set the '[ and '] marks.
+      curbuf->b_op_start.lnum = eap->line1;
+      curbuf->b_op_end.lnum = line2;
+      curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
+    }
 
     if (!global_busy) {
       // when interactive leave cursor on the match
diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c
index f8cf341836..f6d37adf89 100644
--- a/src/nvim/fileio.c
+++ b/src/nvim/fileio.c
@@ -242,6 +242,7 @@ int readfile(char_u *fname, char_u *sfname, linenr_T from, linenr_T lines_to_ski
   bool notconverted = false;             // true if conversion wanted but it wasn't possible
   char_u conv_rest[CONV_RESTLEN];
   int conv_restlen = 0;                 // nr of bytes in conv_rest[]
+  pos_T orig_start;
   buf_T *old_curbuf;
   char_u *old_b_ffname;
   char_u *old_b_fname;
@@ -298,14 +299,10 @@ int readfile(char_u *fname, char_u *sfname, linenr_T from, linenr_T lines_to_ski
   fname = sfname;
 #endif
 
-  /*
-   * The BufReadCmd and FileReadCmd events intercept the reading process by
-   * executing the associated commands instead.
-   */
+  // The BufReadCmd and FileReadCmd events intercept the reading process by
+  // executing the associated commands instead.
   if (!filtering && !read_stdin && !read_buffer) {
-    pos_T pos;
-
-    pos = curbuf->b_op_start;
+    orig_start = curbuf->b_op_start;
 
     // Set '[ mark to the line above where the lines go (line 1 if zero).
     curbuf->b_op_start.lnum = ((from == 0) ? 1 : from);
@@ -335,7 +332,7 @@ int readfile(char_u *fname, char_u *sfname, linenr_T from, linenr_T lines_to_ski
       return aborting() ? FAIL : OK;
     }
 
-    curbuf->b_op_start = pos;
+    curbuf->b_op_start = orig_start;
   }
 
   if ((shortmess(SHM_OVER) || curbuf->b_help) && p_verbose == 0) {
@@ -576,9 +573,8 @@ int readfile(char_u *fname, char_u *sfname, linenr_T from, linenr_T lines_to_ski
 
   ++no_wait_return;         // don't wait for return yet
 
-  /*
-   * Set '[ mark to the line above where the lines go (line 1 if zero).
-   */
+  // Set '[ mark to the line above where the lines go (line 1 if zero).
+  orig_start = curbuf->b_op_start;
   curbuf->b_op_start.lnum = ((from == 0) ? 1 : from);
   curbuf->b_op_start.col = 0;
 
@@ -618,6 +614,7 @@ int readfile(char_u *fname, char_u *sfname, linenr_T from, linenr_T lines_to_ski
     try_mac = (vim_strchr(p_ffs, 'm') != NULL);
     try_dos = (vim_strchr(p_ffs, 'd') != NULL);
     try_unix = (vim_strchr(p_ffs, 'x') != NULL);
+    curbuf->b_op_start = orig_start;
 
     if (msg_scrolled == n) {
       msg_scroll = m;
@@ -1888,13 +1885,13 @@ failed:
     check_cursor_lnum();
     beginline(BL_WHITE | BL_FIX);           // on first non-blank
 
-    /*
-     * Set '[ and '] marks to the newly read lines.
-     */
-    curbuf->b_op_start.lnum = from + 1;
-    curbuf->b_op_start.col = 0;
-    curbuf->b_op_end.lnum = from + linecnt;
-    curbuf->b_op_end.col = 0;
+    if (!cmdmod.lockmarks) {
+      // Set '[ and '] marks to the newly read lines.
+      curbuf->b_op_start.lnum = from + 1;
+      curbuf->b_op_start.col = 0;
+      curbuf->b_op_end.lnum = from + linecnt;
+      curbuf->b_op_end.col = 0;
+    }
   }
   msg_scroll = msg_save;
 
@@ -2252,6 +2249,8 @@ int buf_write(buf_T *buf, char_u *fname, char_u *sfname, linenr_T start, linenr_
   int write_undo_file = FALSE;
   context_sha256_T sha_ctx;
   unsigned int bkc = get_bkc_value(buf);
+  const pos_T orig_start = buf->b_op_start;
+  const pos_T orig_end = buf->b_op_end;
 
   if (fname == NULL || *fname == NUL) {  // safety check
     return FAIL;
@@ -2432,7 +2431,13 @@ int buf_write(buf_T *buf, char_u *fname, char_u *sfname, linenr_T start, linenr_
     if (buf == NULL || (buf->b_ml.ml_mfp == NULL && !empty_memline)
         || did_cmd || nofile_err
         || aborting()) {
-      --no_wait_return;
+      if (buf != NULL && cmdmod.lockmarks) {
+        // restore the original '[ and '] positions
+        buf->b_op_start = orig_start;
+        buf->b_op_end = orig_end;
+      }
+
+      no_wait_return--;
       msg_scroll = msg_save;
       if (nofile_err) {
         emsg(_("E676: No matching autocommands for acwrite buffer"));
@@ -2513,6 +2518,11 @@ int buf_write(buf_T *buf, char_u *fname, char_u *sfname, linenr_T start, linenr_
     }
   }
 
+  if (cmdmod.lockmarks) {
+    // restore the original '[ and '] positions
+    buf->b_op_start = orig_start;
+    buf->b_op_end = orig_end;
+  }
 
   if (shortmess(SHM_OVER) && !exiting) {
     msg_scroll = FALSE;             // overwrite previous file message
diff --git a/src/nvim/ops.c b/src/nvim/ops.c
index e66935db7e..83a7c31991 100644
--- a/src/nvim/ops.c
+++ b/src/nvim/ops.c
@@ -267,14 +267,14 @@ void op_shift(oparg_T *oap, int curs_top, int amount)
     msg_attr_keep((char *)IObuff, 0, true, false);
   }
 
-  /*
-   * Set "'[" and "']" marks.
-   */
-  curbuf->b_op_start = oap->start;
-  curbuf->b_op_end.lnum = oap->end.lnum;
-  curbuf->b_op_end.col = (colnr_T)STRLEN(ml_get(oap->end.lnum));
-  if (curbuf->b_op_end.col > 0) {
-    curbuf->b_op_end.col--;
+  if (!cmdmod.lockmarks) {
+    // Set "'[" and "']" marks.
+    curbuf->b_op_start = oap->start;
+    curbuf->b_op_end.lnum = oap->end.lnum;
+    curbuf->b_op_end.col = (colnr_T)STRLEN(ml_get(oap->end.lnum));
+    if (curbuf->b_op_end.col > 0) {
+      curbuf->b_op_end.col--;
+    }
   }
 
   changed_lines(oap->start.lnum, 0, oap->end.lnum + 1, 0L, true);
@@ -694,9 +694,11 @@ void op_reindent(oparg_T *oap, Indenter how)
                   "%" PRId64 " lines indented ", i),
          (int64_t)i);
   }
-  // set '[ and '] marks
-  curbuf->b_op_start = oap->start;
-  curbuf->b_op_end = oap->end;
+  if (!cmdmod.lockmarks) {
+    // set '[ and '] marks
+    curbuf->b_op_start = oap->start;
+    curbuf->b_op_end = oap->end;
+  }
 }
 
 /*
@@ -1736,13 +1738,15 @@ int op_delete(oparg_T *oap)
   msgmore(curbuf->b_ml.ml_line_count - old_lcount);
 
 setmarks:
-  if (oap->motion_type == kMTBlockWise) {
-    curbuf->b_op_end.lnum = oap->end.lnum;
-    curbuf->b_op_end.col = oap->start.col;
-  } else {
-    curbuf->b_op_end = oap->start;
+  if (!cmdmod.lockmarks) {
+    if (oap->motion_type == kMTBlockWise) {
+      curbuf->b_op_end.lnum = oap->end.lnum;
+      curbuf->b_op_end.col = oap->start.col;
+    } else {
+      curbuf->b_op_end = oap->start;
+    }
+    curbuf->b_op_start = oap->start;
   }
-  curbuf->b_op_start = oap->start;
 
   return OK;
 }
@@ -2007,9 +2011,11 @@ static int op_replace(oparg_T *oap, int c)
   check_cursor();
   changed_lines(oap->start.lnum, oap->start.col, oap->end.lnum + 1, 0L, true);
 
-  // Set "'[" and "']" marks.
-  curbuf->b_op_start = oap->start;
-  curbuf->b_op_end = oap->end;
+  if (!cmdmod.lockmarks) {
+    // Set "'[" and "']" marks.
+    curbuf->b_op_start = oap->start;
+    curbuf->b_op_end = oap->end;
+  }
 
   return OK;
 }
@@ -2078,11 +2084,11 @@ void op_tilde(oparg_T *oap)
     redraw_curbuf_later(INVERTED);
   }
 
-  /*
-   * Set '[ and '] marks.
-   */
-  curbuf->b_op_start = oap->start;
-  curbuf->b_op_end = oap->end;
+  if (!cmdmod.lockmarks) {
+    // Set '[ and '] marks.
+    curbuf->b_op_start = oap->start;
+    curbuf->b_op_end = oap->end;
+  }
 
   if (oap->line_count > p_report) {
     smsg(NGETTEXT("%" PRId64 " line changed",
@@ -2774,14 +2780,14 @@ static void op_yank_reg(oparg_T *oap, bool message, yankreg_T *reg, bool append)
     }
   }
 
-  /*
-   * Set "'[" and "']" marks.
-   */
-  curbuf->b_op_start = oap->start;
-  curbuf->b_op_end = oap->end;
-  if (yank_type == kMTLineWise) {
-    curbuf->b_op_start.col = 0;
-    curbuf->b_op_end.col = MAXCOL;
+  if (!cmdmod.lockmarks) {
+    // Set "'[" and "']" marks.
+    curbuf->b_op_start = oap->start;
+    curbuf->b_op_end = oap->end;
+    if (yank_type == kMTLineWise) {
+      curbuf->b_op_start.col = 0;
+      curbuf->b_op_end.col = MAXCOL;
+    }
   }
 
   return;
@@ -2914,6 +2920,8 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
   char_u *insert_string = NULL;
   bool allocated = false;
   long cnt;
+  const pos_T orig_start = curbuf->b_op_start;
+  const pos_T orig_end = curbuf->b_op_end;
   unsigned int cur_ve_flags = get_ve_flags();
 
   if (flags & PUT_FIXINDENT) {
@@ -3618,6 +3626,10 @@ error:
   curwin->w_set_curswant = TRUE;
 
 end:
+  if (cmdmod.lockmarks) {
+    curbuf->b_op_start = orig_start;
+    curbuf->b_op_end = orig_end;
+  }
   if (allocated) {
     xfree(insert_string);
   }
@@ -3964,7 +3976,7 @@ int do_join(size_t count, int insert_space, int save_undo, int use_formatoptions
   // and setup the array of space strings lengths
   for (t = 0; t < (linenr_T)count; t++) {
     curr = curr_start = ml_get((linenr_T)(curwin->w_cursor.lnum + t));
-    if (t == 0 && setmark) {
+    if (t == 0 && setmark && !cmdmod.lockmarks) {
       // Set the '[ mark.
       curwin->w_buffer->b_op_start.lnum = curwin->w_cursor.lnum;
       curwin->w_buffer->b_op_start.col = (colnr_T)STRLEN(curr);
@@ -4085,7 +4097,7 @@ int do_join(size_t count, int insert_space, int save_undo, int use_formatoptions
 
   ml_replace(curwin->w_cursor.lnum, newp, false);
 
-  if (setmark) {
+  if (setmark && !cmdmod.lockmarks) {
     // Set the '] mark.
     curwin->w_buffer->b_op_end.lnum = curwin->w_cursor.lnum;
     curwin->w_buffer->b_op_end.col = sumsize;
@@ -4223,8 +4235,10 @@ static void op_format(oparg_T *oap, int keep_cursor)
     redraw_curbuf_later(INVERTED);
   }
 
-  // Set '[ mark at the start of the formatted area
-  curbuf->b_op_start = oap->start;
+  if (!cmdmod.lockmarks) {
+    // Set '[ mark at the start of the formatted area
+    curbuf->b_op_start = oap->start;
+  }
 
   // For "gw" remember the cursor position and put it back below (adjusted
   // for joined and split lines).
@@ -4246,8 +4260,10 @@ static void op_format(oparg_T *oap, int keep_cursor)
   old_line_count = curbuf->b_ml.ml_line_count - old_line_count;
   msgmore(old_line_count);
 
-  // put '] mark on the end of the formatted area
-  curbuf->b_op_end = curwin->w_cursor;
+  if (!cmdmod.lockmarks) {
+    // put '] mark on the end of the formatted area
+    curbuf->b_op_end = curwin->w_cursor;
+  }
 
   if (keep_cursor) {
     curwin->w_cursor = saved_cursor;
@@ -4845,7 +4861,7 @@ void op_addsub(oparg_T *oap, linenr_T Prenum1, bool g_cmd)
 
     // Set '[ mark if something changed. Keep the last end
     // position from do_addsub().
-    if (change_cnt > 0) {
+    if (change_cnt > 0 && !cmdmod.lockmarks) {
       curbuf->b_op_start = startpos;
     }
 
@@ -5199,11 +5215,13 @@ int do_addsub(int op_type, pos_T *pos, int length, linenr_T Prenum1)
     }
   }
 
-  // set the '[ and '] marks
-  curbuf->b_op_start = startpos;
-  curbuf->b_op_end = endpos;
-  if (curbuf->b_op_end.col > 0) {
-    curbuf->b_op_end.col--;
+  if (!cmdmod.lockmarks) {
+    // set the '[ and '] marks
+    curbuf->b_op_start = startpos;
+    curbuf->b_op_end = endpos;
+    if (curbuf->b_op_end.col > 0) {
+      curbuf->b_op_end.col--;
+    }
   }
 
 theend:
@@ -6016,6 +6034,8 @@ static void op_function(const oparg_T *oap)
 {
   const TriState save_virtual_op = virtual_op;
   const bool save_finish_op = finish_op;
+  const pos_T orig_start = curbuf->b_op_start;
+  const pos_T orig_end = curbuf->b_op_end;
 
   if (*p_opfunc == NUL) {
     emsg(_("E774: 'operatorfunc' is empty"));
@@ -6049,6 +6069,10 @@ static void op_function(const oparg_T *oap)
 
     virtual_op = save_virtual_op;
     finish_op = save_finish_op;
+    if (cmdmod.lockmarks) {
+      curbuf->b_op_start = orig_start;
+      curbuf->b_op_end = orig_end;
+    }
   }
 }
 
diff --git a/src/nvim/testdir/test_autocmd.vim b/src/nvim/testdir/test_autocmd.vim
index 45285b69a1..231ab2acf1 100644
--- a/src/nvim/testdir/test_autocmd.vim
+++ b/src/nvim/testdir/test_autocmd.vim
@@ -2380,6 +2380,40 @@ func Test_autocmd_was_using_freed_memory()
   pclose
 endfunc
 
+func Test_BufWrite_lockmarks()
+  edit! Xtest
+  call setline(1, ['a', 'b', 'c', 'd'])
+
+  " :lockmarks preserves the marks
+  call SetChangeMarks(2, 3)
+  lockmarks write
+  call assert_equal([2, 3], [line("'["), line("']")])
+
+  " *WritePre autocmds get the correct line range, but lockmarks preserves the
+  " original values for the user
+  augroup lockmarks
+    au!
+    au BufWritePre,FilterWritePre * call assert_equal([1, 4], [line("'["), line("']")])
+    au FileWritePre * call assert_equal([3, 4], [line("'["), line("']")])
+  augroup END
+
+  lockmarks write
+  call assert_equal([2, 3], [line("'["), line("']")])
+
+  if executable('cat')
+    lockmarks %!cat
+    call assert_equal([2, 3], [line("'["), line("']")])
+  endif
+
+  lockmarks 3,4write Xtest2
+  call assert_equal([2, 3], [line("'["), line("']")])
+
+  au! lockmarks
+  augroup! lockmarks
+  call delete('Xtest')
+  call delete('Xtest2')
+endfunc
+
 " FileChangedShell tested in test_filechanged.vim
 
 func LogACmd()
diff --git a/src/nvim/testdir/test_diffmode.vim b/src/nvim/testdir/test_diffmode.vim
index 3a0c615cf6..fe928c0f65 100644
--- a/src/nvim/testdir/test_diffmode.vim
+++ b/src/nvim/testdir/test_diffmode.vim
@@ -1146,6 +1146,25 @@ func Test_diff_followwrap()
   bwipe!
 endfunc
 
+func Test_diff_maintains_change_mark()
+  enew!
+  call setline(1, ['a', 'b', 'c', 'd'])
+  diffthis
+  new
+  call setline(1, ['a', 'b', 'c', 'e'])
+  " Set '[ and '] marks
+  2,3yank
+  call assert_equal([2, 3], [line("'["), line("']")])
+  " Verify they aren't affected by the implicit diff
+  diffthis
+  call assert_equal([2, 3], [line("'["), line("']")])
+  " Verify they aren't affected by an explicit diff
+  diffupdate
+  call assert_equal([2, 3], [line("'["), line("']")])
+  bwipe!
+  bwipe!
+endfunc
+
 func Test_diff_rnu()
   CheckScreendump