Fix various issues with SpawnMultiCursor{Up,Down} (#3145)

* SpawnMultiCursorUp/Down: change order of adding cursors

SpawnMultiCursor{Up,Down} currently works in a tricky way: instead of
creating a new cursor above or below, it moves the current "primary"
cursor above or below, and then creates a new cursor below or above the
new position of the current cursor (i.e. at its previous position),
creating an illusion for the user that the current (top-most or
bottom-most) cursor is a newly spawned cursor.

This trick causes at least the following issues:

- When the line above or below, where we spawn a new cursor, is shorter
  than the current cursor position in the current line, the new cursor
  is placed at the end of this short line (which is expected), but also
  the current cursor unexpectedly changes its x position and moves
  below/above the new cursor.

- When removing a cursor in RemoveMultiCursor (default Alt-p key), it
  non-intuitively removes the cursor which, from the user point of view,
  is not the last but the last-but-one cursor.

Fix these issues by replacing the trick with a straightforward logic:
just create the new cursor above or below the last one.

Note that this fix has a user-visible side effect: the last cursor is
no longer the "primary" one (since it is now the last in the list, not
the first), so e.g. when the user clears multicursors via Esc key, the
remaining cursor is the first one, not the last one. I assume it's ok.

* SpawnMultiCursorUp/Down: move common code to a helper fn

* SpawnMultiCursorUp/Down: honor visual width and LastVisualX

Make spawning multicursors up/down behave more similarly to cursor
movements up/down. This change fixes 2 issues at once:

- SpawnMultiCursorUp/Down doesn't take into account the visual width of
  the text before the cursor, which may be different from its character
  width (e.g. if it contains tabs). So e.g. if the number of tabs before
  the cursor in the current line is not the same as in the new line, the
  new cursor is placed at an unexpected location.

- SpawnMultiCursorUp/Down doesn't take into account the cursor's
  remembered x position (LastVisualX) when e.g. spawning a new cursor
  in the below line which is short than the current cursor position, and
  then spawning yet another cursor in the next below line which is
  longer than this short line.

* SpawnMultiCursorUp/Down: honor softwrap

When softwrap is enabled and the current line is wrapped, make
SpawnMultiCursor{Up,Down} spawn cursor in the next visual line within
this wrapped line, similarly to how we handle cursor movements up/down
within wrapped lines.

* SpawnMultiCursorUp/Down: deselect when spawning cursors

To avoid weird user experience (spawned cursors messing with selections
of existing cursors).

internal/action/actions.go

@@ -1761,15 +1761,39 @@ func (h *BufPane) SpawnMultiCursor() bool {
 	return true
 }
 
-// SpawnMultiCursorUp creates additional cursor, at the same X (if possible), one Y less.
-func (h *BufPane) SpawnMultiCursorUp() bool {
-	if h.Cursor.Y == 0 {
-		return false
+// SpawnMultiCursorUpN is not an action
+func (h *BufPane) SpawnMultiCursorUpN(n int) bool {
+	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
+	var c *buffer.Cursor
+	if !h.Buf.Settings["softwrap"].(bool) {
+		if n > 0 && lastC.Y == 0 {
+			return false
+		}
+		if n < 0 && lastC.Y+1 == h.Buf.LinesNum() {
+			return false
+		}
+
+		h.Buf.DeselectCursors()
+
+		c = buffer.NewCursor(h.Buf, buffer.Loc{lastC.X, lastC.Y - n})
+		c.LastVisualX = lastC.LastVisualX
+		c.X = c.GetCharPosInLine(h.Buf.LineBytes(c.Y), c.LastVisualX)
+		c.Relocate()
+	} else {
+		vloc := h.VLocFromLoc(lastC.Loc)
+		sloc := h.Scroll(vloc.SLoc, -n)
+		if sloc == vloc.SLoc {
+			return false
+		}
+
+		h.Buf.DeselectCursors()
+
+		vloc.SLoc = sloc
+		vloc.VisualX = lastC.LastVisualX
+		c = buffer.NewCursor(h.Buf, h.LocFromVLoc(vloc))
+		c.LastVisualX = lastC.LastVisualX
 	}
-	h.Cursor.GotoLoc(buffer.Loc{h.Cursor.X, h.Cursor.Y - 1})
-	h.Cursor.Relocate()
 
-	c := buffer.NewCursor(h.Buf, buffer.Loc{h.Cursor.X, h.Cursor.Y + 1})
 	h.Buf.AddCursor(c)
 	h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
 	h.Buf.MergeCursors()
@@ -1778,20 +1802,14 @@ func (h *BufPane) SpawnMultiCursorUp() bool {
 	return true
 }
 
+// SpawnMultiCursorUp creates additional cursor, at the same X (if possible), one Y less.
+func (h *BufPane) SpawnMultiCursorUp() bool {
+	return h.SpawnMultiCursorUpN(1)
+}
+
 // SpawnMultiCursorDown creates additional cursor, at the same X (if possible), one Y more.
 func (h *BufPane) SpawnMultiCursorDown() bool {
-	if h.Cursor.Y+1 == h.Buf.LinesNum() {
-		return false
-	}
-	h.Cursor.GotoLoc(buffer.Loc{h.Cursor.X, h.Cursor.Y + 1})
-	h.Cursor.Relocate()
-
-	c := buffer.NewCursor(h.Buf, buffer.Loc{h.Cursor.X, h.Cursor.Y - 1})
-	h.Buf.AddCursor(c)
-	h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
-	h.Buf.MergeCursors()
-	h.Relocate()
-	return true
+	return h.SpawnMultiCursorUpN(-1)
 }
 
 // SpawnMultiCursorSelect adds a cursor at the beginning of each line of a selection

internal/buffer/buffer.go

@@ -568,6 +568,13 @@ func (b *Buffer) RelocateCursors() {
 	}
 }
 
+// DeselectCursors removes selection from all cursors
+func (b *Buffer) DeselectCursors() {
+	for _, c := range b.cursors {
+		c.Deselect(true)
+	}
+}
+
 // RuneAt returns the rune at a given location in the buffer
 func (b *Buffer) RuneAt(loc Loc) rune {
 	line := b.LineBytes(loc.Y)