From 9fdea8254250b198e02cfdcddde5d4f059900e67 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Mon, 4 Mar 2024 22:23:50 +0100 Subject: [PATCH] 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 | 56 +++++++++++++++++++++++++------------- internal/buffer/buffer.go | 7 +++++ 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index bfbf2354f2..ea0255f95d 100644 --- a/internal/action/actions.go +++ b/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 diff --git a/internal/buffer/buffer.go b/internal/buffer/buffer.go index dc4d037f1c..7955e0bb69 100644 --- a/internal/buffer/buffer.go +++ b/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)