put fixup, esp. ". register close #5709 #5781

Note some bugs were judged to have too ugly a fix to solve, tests to
demonstrate these problems, and the explanation behind not fixing them
are below.

describe('register . problems', function()
  before_each(reset)

  -- The difficulty here is: The basic requirement is that the text
  -- inserted is treated as if it were typed in insert mode. This is why
  -- the paste method is to enter insert mode and enter the ". register
  -- into readbuf1.
  -- We can't add a count into the readbuf here because the insert mode
  -- count is implemented with readbuf2 which is checked for characters
  -- after readbuf1.
  -- Hence, the ".gp command (which adds extra characters into readbuf1
  -- to emulate leaving the cursor after the text by moving the cursor
  -- after inserting the text) would insert the motion characters into
  -- the buffer instead of using them to move after the insert has been
  -- done.
  -- I could probably get this working properly with a special flag put
  -- into start_redo_ins() and set in do_put(), but I think this adds
  -- much more complexity than fixing this bug justifies.
  pending('should not change the ". register with ".2p', function()
    local orig_register = funcs.getreg('.')
    feed('2".p')
    eq(orig_register, funcs.getreg('.'))
  end)

  describe("cursor positioning after undo and redo with '.'", function()
    before_each(reset)
    local function make_cursor_test(macro_string)
      return function()
        feed(macro_string)
        local afterpos = funcs.getcurpos()
        local orig_string = curbuf_contents()
        feed('u.')
        eq(afterpos, funcs.getcurpos())
        expect(orig_string)
      end
    end
    -- The difficulty here is: setting the cursor after the end of the
    -- pasted text is done by adding a motion command to the
    -- stuffbuffer after the insert.
    -- Modifying 'redobuff' is done in the code that handles inserting
    -- text and moving around.
    -- I could add a special case in ins_esc() that checks for a flag
    -- set in do_put() to add the motion character to the redo buffer,
    -- but I think that is starting to get way too convoluted for the
    -- benefit.
    pending('should be the same after ".gp and ".gpu.',
            make_cursor_test('".gp'))
    -- The difficulty here is: putting forwards is implemented by using
    -- 'a' instead of 'i' to start insert.
    -- Undoing with 'u' an insert that began with 'a' leaves the cursor
    -- where the first character was inserted, not where the cursor was
    -- when the 'a' was pressed.
    -- We account for this the first time by saving the cursor position
    -- in do_put(), but this isn't stored in redobuff for a second time
    -- around.
    -- We can't change how such a fundamental action as undo after
    -- inserting with 'a' behaves, we could add in a special case
    -- whereby we set a flag in do_put() and read it when entering
    -- insert mode but this seems like way too much to fix such a minor
    -- bug.
    pending('should be the same after ".pu. and ".pu.u.',
            make_cursor_test('".pu.'))
  end)
end)

2 files changed, 107 insertions, 44 deletions

diff --git a/src/nvim/normal.c b/src/nvim/normal.c
index 227bfbe779..9db02de2a6 100644
--- a/src/nvim/normal.c
+++ b/src/nvim/normal.c
@@ -1517,10 +1517,7 @@ void do_pending_operator(cmdarg_T *cap, int old_col, bool gui_yank)
         coladvance(curwin->w_curswant);
       }
       cap->count0 = redo_VIsual_count;
-      if (redo_VIsual_count != 0)
-        cap->count1 = redo_VIsual_count;
-      else
-        cap->count1 = 1;
+      cap->count1 = (cap->count0 == 0 ? 1 : cap->count0);
     } else if (VIsual_active) {
       if (!gui_yank) {
         /* Save the current VIsual area for '< and '> marks, and "gv" */
@@ -7727,16 +7724,22 @@ static void nv_put(cmdarg_T *cap)
         savereg = copy_register(regname);
       }
 
-      /* Now delete the selected text. */
-      cap->cmdchar = 'd';
-      cap->nchar = NUL;
-      cap->oap->regname = NUL;
-      nv_operator(cap);
-      do_pending_operator(cap, 0, false);
-      empty = (curbuf->b_ml.ml_flags & ML_EMPTY);
+      // To place the cursor correctly after a blockwise put, and to leave the
+      // text in the correct position when putting over a selection with
+      // 'virtualedit' and past the end of the line, we use the 'c' operator in
+      // do_put(), which requires the visual selection to still be active.
+      if (!VIsual_active || VIsual_mode == 'V' || regname != '.') {
+        // Now delete the selected text.
+        cap->cmdchar = 'd';
+        cap->nchar = NUL;
+        cap->oap->regname = NUL;
+        nv_operator(cap);
+        do_pending_operator(cap, 0, false);
+        empty = (curbuf->b_ml.ml_flags & ML_EMPTY);
 
-      /* delete PUT_LINE_BACKWARD; */
-      cap->oap->regname = regname;
+        // delete PUT_LINE_BACKWARD;
+        cap->oap->regname = regname;
+      }
 
       /* When deleted a linewise Visual area, put the register as
        * lines to avoid it joined with the next line.  When deletion was
diff --git a/src/nvim/ops.c b/src/nvim/ops.c
index 10d6be85f8..645dcc0865 100644
--- a/src/nvim/ops.c
+++ b/src/nvim/ops.c
@@ -2637,12 +2637,81 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
    * special characters (newlines, etc.).
    */
   if (regname == '.') {
-    (void)stuff_inserted((dir == FORWARD ? (count == -1 ? 'o' : 'a') :
-                          (count == -1 ? 'O' : 'i')), count, FALSE);
-    /* Putting the text is done later, so can't really move the cursor to
-     * the next character.  Use "l" to simulate it. */
-    if ((flags & PUT_CURSEND) && gchar_cursor() != NUL)
-      stuffcharReadbuff('l');
+    bool non_linewise_vis = (VIsual_active && VIsual_mode != 'V');
+
+    // PUT_LINE has special handling below which means we use 'i' to start.
+    char command_start_char = non_linewise_vis ? 'c' :
+      (flags & PUT_LINE ? 'i' : (dir == FORWARD ? 'a' : 'i'));
+
+    // To avoid being the affect of 'autoindent' on linewise puts, we create a
+    // new line with `:put _`.
+    if (flags & PUT_LINE) {
+      do_put('_', NULL, dir, 1, PUT_LINE);
+    }
+
+    // If given a count when putting linewise, we stuff the readbuf with the
+    // dot register 'count' times split by newlines.
+    if (flags & PUT_LINE) {
+      stuffcharReadbuff(command_start_char);
+      for (; count > 0; count--) {
+        (void)stuff_inserted(NUL, 1, count != 1);
+        if (count != 1) {
+          // To avoid 'autoindent' affecting the text, use Ctrl_U to remove any
+          // whitespace. Can't just insert Ctrl_U into readbuf1, this would go
+          // back to the previous line in the case of 'noautoindent' and
+          // 'backspace' includes "eol". So we insert a dummy space for Ctrl_U
+          // to consume.
+          stuffReadbuff((char_u *)"\n ");
+          stuffcharReadbuff(Ctrl_U);
+        }
+      }
+    } else {
+      (void)stuff_inserted(command_start_char, count, false);
+    }
+    // Putting the text is done later, so can't move the cursor to the next
+    // character.  Simulate it with motion commands after the insert.
+    if (flags & PUT_CURSEND) {
+      if (flags & PUT_LINE) {
+        stuffReadbuff((char_u *)"j0");
+      } else {
+        // Avoid ringing the bell from attempting to move into the space after
+        // the current line. We can stuff the readbuffer with "l" if:
+        // 1) 'virtualedit' is "all" or "onemore"
+        // 2) We are not at the end of the line
+        // 3) We are not  (one past the end of the line && on the last line)
+        //    This allows a visual put over a selection one past the end of the
+        //    line joining the current line with the one below.
+
+        // curwin->w_cursor.col marks the byte position of the cursor in the
+        // currunt line. It increases up to a max of
+        // STRLEN(ml_get(curwin->w_cursor.lnum)). With 'virtualedit' and the
+        // cursor past the end of the line, curwin->w_cursor.coladd is
+        // incremented instead of curwin->w_cursor.col.
+        char_u *cursor_pos = get_cursor_pos_ptr();
+        bool one_past_line = (*cursor_pos == NUL);
+        bool end_of_line = false;
+        if (!one_past_line) {
+          end_of_line = (*(cursor_pos + mb_ptr2len(cursor_pos)) == NUL);
+        }
+
+        bool virtualedit_allows = (ve_flags == VE_ALL
+                                   || ve_flags == VE_ONEMORE);
+        bool end_of_file = (
+            (curbuf->b_ml.ml_line_count == curwin->w_cursor.lnum)
+            && one_past_line);
+        if (virtualedit_allows || !(end_of_line || end_of_file)) {
+          stuffcharReadbuff('l');
+        }
+      }
+    } else if (flags & PUT_LINE) {
+      stuffReadbuff((char_u *)"g'[");
+    }
+
+    // So the 'u' command restores cursor position after ".p, save the cursor
+    // position now (though not saving any text).
+    if (command_start_char == 'a') {
+      u_save(curwin->w_cursor.lnum, curwin->w_cursor.lnum + 1);
+    }
     return;
   }
 
@@ -2831,14 +2900,12 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
       else
         getvcol(curwin, &curwin->w_cursor, NULL, NULL, &col);
 
-      if (has_mbyte)
-        /* move to start of next multi-byte character */
-        curwin->w_cursor.col += (*mb_ptr2len)(get_cursor_pos_ptr());
-      else if (c != TAB || ve_flags != VE_ALL)
-        ++curwin->w_cursor.col;
-      ++col;
-    } else
+      // move to start of next multi-byte character
+      curwin->w_cursor.col += (*mb_ptr2len)(get_cursor_pos_ptr());
+      col++;
+    } else {
       getvcol(curwin, &curwin->w_cursor, &col, NULL, &endcol2);
+    }
 
     col += curwin->w_cursor.coladd;
     if (ve_flags == VE_ALL
@@ -2892,8 +2959,7 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
         bd.startspaces = incr - bd.endspaces;
         --bd.textcol;
         delcount = 1;
-        if (has_mbyte)
-          bd.textcol -= (*mb_head_off)(oldp, oldp + bd.textcol);
+        bd.textcol -= (*mb_head_off)(oldp, oldp + bd.textcol);
         if (oldp[bd.textcol] != TAB) {
           /* Only a Tab can be split into spaces.  Other
            * characters will have to be moved to after the
@@ -2975,21 +3041,13 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
       // if type is kMTCharWise, FORWARD is the same as BACKWARD on the next
       // char
       if (dir == FORWARD && gchar_cursor() != NUL) {
-        if (has_mbyte) {
-          int bytelen = (*mb_ptr2len)(get_cursor_pos_ptr());
-
-          /* put it on the next of the multi-byte character. */
-          col += bytelen;
-          if (yanklen) {
-            curwin->w_cursor.col += bytelen;
-            curbuf->b_op_end.col += bytelen;
-          }
-        } else {
-          ++col;
-          if (yanklen) {
-            ++curwin->w_cursor.col;
-            ++curbuf->b_op_end.col;
-          }
+        int bytelen = (*mb_ptr2len)(get_cursor_pos_ptr());
+
+        // put it on the next of the multi-byte character.
+        col += bytelen;
+        if (yanklen) {
+          curwin->w_cursor.col += bytelen;
+          curbuf->b_op_end.col += bytelen;
         }
       }
       curbuf->b_op_start = curwin->w_cursor;
@@ -3027,7 +3085,9 @@ void do_put(int regname, yankreg_T *reg, int dir, long count, int flags)
         }
         if (VIsual_active)
           lnum++;
-      } while (VIsual_active && lnum <= curbuf->b_visual.vi_end.lnum);
+      } while (VIsual_active
+               && (lnum <= curbuf->b_visual.vi_end.lnum
+                   || lnum <= curbuf->b_visual.vi_start.lnum));
 
       if (VIsual_active) {  /* reset lnum to the last visual line */
         lnum--;